Revert "Register the updater app id in registry (Windows)."

This reverts commit 08aba31.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 712315 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzA4YWJhMzE1MmJjMjk3ZTFlMzAwZGEyOWZkYWE2M2NjOTY3MTAzMDcM

Sample Failed Build: https://ci.chromium.org/b/8897675907369267936

Sample Failed Step: compile

Original change's description:
> Register the updater app id in registry (Windows).
> 
> Send both the updater and Chrome app ids in the "update apps" use case.
> With this change, --install and --ua commands work as expected, meaning
> the app ids and the versions are sent during update checks.
> 
> There is a edge case, not handled currently, where the app id for
> the updater is not available until the first update check has happened.
> This is going to be resolved in a future CL which lands as part of
> fixing this CR bug.
> 
> R=waffles
> 
> Bug: 1020285
> Change-Id: I2856a795ed61f590f85d792e4d5e1493bd23b84f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894833
> Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
> Commit-Queue: Sorin Jianu <sorin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#712315}


(cherry picked from commit 03597a4)

Change-Id: I82dcc710be51b507c48a66de5dcb3b9d48a50c44
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1020285
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1898537
Cr-Original-Commit-Position: refs/heads/master@{#712380}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900126
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/3960@{#3}
Cr-Branched-From: c91d5bb-refs/heads/master@{#712352}

chrome/updater/installer.cc

@@ -57,21 +57,6 @@ update_client::CrxComponent Installer::MakeCrxComponent() {
   return component;
 }
 
-std::vector<std::string> Installer::FindAppIds() {
-  base::FilePath app_install_dir;
-  if (!GetProductDirectory(&app_install_dir))
-    return {};
-  app_install_dir = app_install_dir.AppendASCII(kAppsDir);
-  std::vector<std::string> app_ids;
-  base::FileEnumerator file_enumerator(app_install_dir, false,
-                                       base::FileEnumerator::DIRECTORIES);
-  for (auto path = file_enumerator.Next(); !path.value().empty();
-       path = file_enumerator.Next()) {
-    app_ids.push_back(path.BaseName().MaybeAsASCII());
-  }
-  return app_ids;
-}
-
 void Installer::FindInstallOfApp() {
   VLOG(1) << __func__ << " for " << app_id_;
 

chrome/updater/installer.h

@@ -40,9 +40,6 @@ class Installer final : public update_client::CrxInstaller {
 
   const std::string app_id() const { return app_id_; }
 
-  // Returns the app ids that are managed by the CRX installer.
-  static std::vector<std::string> FindAppIds();
-
   // Finds the highest version install of the app, and updates the install
   // info for this installer instance.
   void FindInstallOfApp();

chrome/updater/update_apps.cc

@@ -54,21 +54,9 @@ class Observer : public update_client::UpdateClient::Observer {
 }  // namespace
 
 int UpdateApps() {
-  auto app_ids = Installer::FindAppIds();
-
-  // Include the app id for the updater if it is not found. This could happen
-  // before the first update for the updater has been handled. This is a
-  // temporary workaround until the source of truth for the registered
-  // version is resolved.
-  if (!base::Contains(app_ids, kUpdaterAppId))
-    app_ids.push_back(kUpdaterAppId);
-
-  std::vector<base::Optional<update_client::CrxComponent>> components;
-  for (const auto& app_id : app_ids) {
-    auto installer = base::MakeRefCounted<Installer>(app_id);
-    installer->FindInstallOfApp();
-    components.push_back(installer->MakeCrxComponent());
-  }
+  auto installer = base::MakeRefCounted<Installer>(kUpdaterAppId);
+  installer->FindInstallOfApp();
+  const auto component = installer->MakeCrxComponent();
 
   base::SingleThreadTaskExecutor main_task_executor(base::MessagePumpType::UI);
   base::RunLoop runloop;
@@ -83,16 +71,17 @@ int UpdateApps() {
     Observer observer(update_client);
     update_client->AddObserver(&observer);
 
+    const std::vector<std::string> ids = {installer->app_id()};
     update_client->Update(
-        app_ids,
+        ids,
         base::BindOnce(
-            [](const std::vector<base::Optional<update_client::CrxComponent>>&
-                   components,
-               const std::vector<std::string>& ids) {
-              DCHECK_EQ(components.size(), ids.size());
-              return components;
+            [](const update_client::CrxComponent& component,
+               const std::vector<std::string>& ids)
+                -> std::vector<base::Optional<update_client::CrxComponent>> {
+              DCHECK_EQ(1u, ids.size());
+              return {component};
             },
-            components),
+            component),
         false,
         base::BindOnce(
             [](base::OnceClosure closure, update_client::Error error) {

chrome/updater/updater.cc

@@ -23,16 +23,13 @@
 #include "chrome/updater/win/setup/uninstall.h"
 #endif
 
-// To install the updater on Windows, run "updatersetup.exe" from the
-// build directory.
-//
-// To uninstall, run "updater.exe --uninstall" from its install directory,
-// which is under %LOCALAPPDATA%\Google\GoogleUpdater, or from the |out|
-// directory of the build.
-//
-// To debug, use the command line arguments:
-//    --enable-logging --vmodule=*/chrome/updater/*=2.
-
+// To install the updater on Windows, run:
+// "updater.exe --install --enable-logging --v=1 --vmodule=*/chrome/updater/*"
+// from the build directory. The program needs a number of dependencies which
+// are available in the |out| directory of the build.
+// To uninstall, run "updater.exe --uninstall" from its install directory or
+// from the build out directory. Doing this will make the program delete its
+// install directory using a shim cmd script.
 namespace updater {
 
 namespace {
@@ -89,7 +86,8 @@ int UpdaterUpdateApps() {
 int UpdaterInstallApp() {
 #if defined(OS_WIN)
   // TODO(sorin): pick up the app id from the tag. https://crbug.com/1014298
-  return InstallApp({kChromeAppId});
+  // For now, use Omaha4 app id, just to get a CRX for testing of the UI.
+  return InstallApp({kUpdaterAppId});
 #else
   NOTREACHED();
   return -1;

chrome/updater/updater_constants.cc

@@ -20,6 +20,7 @@ const char kTestSwitch[] = "test";
 const char kInitDoneNotifierSwitch[] = "init-done-notifier";
 const char kNoRateLimitSwitch[] = "no-rate-limit";
 const char kEnableLoggingSwitch[] = "enable-logging";
+const char kLoggingLevelSwitch[] = "v";
 const char kLoggingModuleSwitch[] = "vmodule";
 
 // URLs.

chrome/updater/updater_constants.h

@@ -44,6 +44,9 @@ extern const char kInitDoneNotifierSwitch[];
 // Enables logging.
 extern const char kEnableLoggingSwitch[];
 
+// Specifies the logging level.
+extern const char kLoggingLevelSwitch[];
+
 // Specifies the logging module filter.
 extern const char kLoggingModuleSwitch[];
 
@@ -56,7 +59,7 @@ extern const char kUpdaterJSONDefaultUrl[];
 extern const char kCrashUploadURL[];
 extern const char kCrashStagingUploadURL[];
 
-// File system paths.
+// Paths.
 //
 // The directory name where CRX apps get installed. This is provided for demo
 // purposes, since products installed by this updater will be installed in

chrome/updater/win/constants.cc

@@ -6,9 +6,7 @@
 
 namespace updater {
 
+// The prefix to use for global names in WIN32 API's.
 const base::char16 kGlobalPrefix[] = L"Global\\G";
 
-extern const base::char16 kRegistryValuePV[] = L"pv";
-extern const base::char16 kRegistryValueName[] = L"name";
-
 }  // namespace updater

chrome/updater/win/constants.h

@@ -6,25 +6,13 @@
 #define CHROME_UPDATER_WIN_CONSTANTS_H_
 
 #include "base/strings/string16.h"
-#include "chrome/updater/updater_version.h"
 
 namespace updater {
 
 // The prefix to use for global names in WIN32 API's. The prefix is necessary
 // to avoid collision on kernel object names.
 extern const base::char16 kGlobalPrefix[];
 
-// Registry keys and value names.
-#define COMPANY_KEY "Software\\" COMPANY_SHORTNAME_STRING "\\"
-// Use |Update| instead of PRODUCT_FULLNAME_STRING for the registry key name
-// to be backward compatible with Google Update / Omaha.
-#define UPDATER_KEY COMPANY_KEY "Update\\"
-#define CLIENTS_KEY UPDATER_KEY "Clients\\"
-#define CLIENT_STATE_KEY UPDATER_KEY "ClientState\\"
-
-extern const base::char16 kRegistryValuePV[];
-extern const base::char16 kRegistryValueName[];
-
 }  // namespace updater
 
 #endif  // CHROME_UPDATER_WIN_CONSTANTS_H_

chrome/updater/win/installer/create_installer_archive.py

@@ -256,10 +256,6 @@ def DoComponentBuildTasks(staging_dir, build_dir, setup_runtime_deps):
     g_archive_inputs.append(setup_component_dll)
     shutil.copy(setup_component_dll, installer_dir)
 
-  # Handle the ICU data file dependency.
-  g_archive_inputs.append("icudtl.dat")
-  shutil.copy("icudtl.dat", installer_dir)
-
 def main(options):
   """Main method that reads input file, creates archive file and writes
   resource input file.

chrome/updater/win/installer/installer.cc

@@ -208,8 +208,7 @@ ProcessExitResult RunSetup(const Configuration& configuration,
     return ProcessExitResult(COMMAND_STRING_OVERFLOW);
   }
 
-  if (!cmd_line.append(
-          L" --install --enable-logging --vmodule=*/chrome/updater/*=2"))
+  if (!cmd_line.append(L" --install --enable-logging --v=1"))
     return ProcessExitResult(COMMAND_STRING_OVERFLOW);
 
   return RunProcessAndWait(setup_exe.get(), cmd_line.get());

chrome/updater/win/net/network_winhttp.cc

@@ -212,9 +212,6 @@ scoped_hinternet NetworkFetcherWinHTTP::OpenRequest() {
 
 HRESULT NetworkFetcherWinHTTP::SendRequest(const std::string& data) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  VLOG(2) << data;
-
   const uint32_t bytes_to_send = base::saturated_cast<uint32_t>(data.size());
   void* request_body =
       bytes_to_send ? const_cast<char*>(data.c_str()) : WINHTTP_NO_REQUEST_DATA;
@@ -235,7 +232,7 @@ void NetworkFetcherWinHTTP::SendRequestComplete() {
       request_handle_.get(),
       WINHTTP_QUERY_RAW_HEADERS_CRLF | WINHTTP_QUERY_FLAG_REQUEST_HEADERS,
       WINHTTP_HEADER_NAME_BY_INDEX, &all);
-  VLOG(3) << "request headers: " << all;
+  VLOG(2) << "request headers: " << all;
 
   net_error_ = ReceiveResponse();
   if (FAILED(net_error_))
@@ -255,7 +252,7 @@ void NetworkFetcherWinHTTP::ReceiveResponseComplete() {
   base::string16 all;
   QueryHeadersString(request_handle_.get(), WINHTTP_QUERY_RAW_HEADERS_CRLF,
                      WINHTTP_HEADER_NAME_BY_INDEX, &all);
-  VLOG(3) << "response headers: " << all;
+  VLOG(2) << "response headers: " << all;
 
   int response_code = 0;
   net_error_ = QueryHeadersInt(request_handle_.get(), WINHTTP_QUERY_STATUS_CODE,
@@ -521,7 +518,7 @@ void NetworkFetcherWinHTTP::StatusCallback(HINTERNET handle,
     base::StringAppendF(&msg, ", info=%s",
                         base::SysWideToUTF8(info_string).c_str());
 
-  VLOG(3) << "WinHttp status callback:"
+  VLOG(2) << "WinHttp status callback:"
           << " handle=" << handle << ", " << msg;
 
   switch (status) {

chrome/updater/win/setup/setup.cc

@@ -15,33 +15,28 @@
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/win/scoped_com_initializer.h"
+#include "chrome/installer/util/copy_tree_work_item.h"
 #include "chrome/installer/util/self_cleaning_temp_dir.h"
 #include "chrome/installer/util/work_item_list.h"
 #include "chrome/updater/updater_constants.h"
 #include "chrome/updater/util.h"
-#include "chrome/updater/win/constants.h"
 #include "chrome/updater/win/setup/setup_util.h"
 #include "chrome/updater/win/task_scheduler.h"
-#include "chrome/updater/win/util.h"
 
 namespace updater {
 
 namespace {
 
 const base::char16* kUpdaterFiles[] = {
-    L"icudtl.dat",
     L"updater.exe",
     L"uninstall.cmd",
 #if defined(COMPONENT_BUILD)
     // TODO(sorin): get the list of component dependencies from a build-time
     // file instead of hardcoding the names of the components here.
     L"base.dll",
-    L"base_i18n.dll",
     L"boringssl.dll",
     L"crcrypto.dll",
-    L"icui18n.dll",
     L"icuuc.dll",
     L"libc++.dll",
     L"prefs.dll",
@@ -95,28 +90,18 @@ int Setup() {
   for (const auto* file : kUpdaterFiles) {
     const base::FilePath target_path = product_dir.Append(file);
     const base::FilePath source_path = source_dir.Append(file);
-    install_list->AddCopyTreeWorkItem(source_path.value(), target_path.value(),
-                                      temp_dir.value(), WorkItem::ALWAYS);
-  }
-
-  for (const auto& key_path :
-       {GetRegistryKeyClientsUpdater(), GetRegistryKeyClientStateUpdater()}) {
-    install_list->AddCreateRegKeyWorkItem(HKEY_CURRENT_USER, key_path,
-                                          WorkItem::kWow64Default);
-    install_list->AddSetRegValueWorkItem(
-        HKEY_CURRENT_USER, key_path, WorkItem::kWow64Default, kRegistryValuePV,
-        base::ASCIIToUTF16(UPDATER_VERSION_STRING), true);
-    install_list->AddSetRegValueWorkItem(
-        HKEY_CURRENT_USER, key_path, WorkItem::kWow64Default,
-        kRegistryValueName, base::ASCIIToUTF16(PRODUCT_FULLNAME_STRING), true);
+    install_list->AddWorkItem(
+        WorkItem::CreateCopyTreeWorkItem(source_path, target_path, temp_dir,
+                                         WorkItem::ALWAYS, base::FilePath()));
   }
 
   base::CommandLine run_updater_ua_command(product_dir.Append(L"updater.exe"));
   run_updater_ua_command.AppendSwitch(kUpdateAppsSwitch);
 #if !defined(NDEBUG)
   run_updater_ua_command.AppendSwitch(kEnableLoggingSwitch);
+  run_updater_ua_command.AppendSwitchASCII(kLoggingLevelSwitch, "1");
   run_updater_ua_command.AppendSwitchASCII(kLoggingModuleSwitch,
-                                           "*/chrome/updater/*=2");
+                                           "*/chrome/updater/*");
 #endif
   if (!install_list->Do() || !RegisterUpdateAppsTask(run_updater_ua_command)) {
     LOG(ERROR) << "Install failed, rolling back...";

chrome/updater/win/setup/uninstall.cc

@@ -16,21 +16,17 @@
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/win/scoped_com_initializer.h"
-#include "chrome/installer/util/work_item_list.h"
 #include "chrome/updater/updater_constants.h"
 #include "chrome/updater/util.h"
-#include "chrome/updater/win/constants.h"
 #include "chrome/updater/win/setup/setup_util.h"
 #include "chrome/updater/win/task_scheduler.h"
 
 namespace updater {
 
 // Reverses the changes made by setup. This is a best effort uninstall:
-// 1. Deletes the scheduled task.
-// 2. Deletes the Clients and ClientState keys.
-// 3. Runs the uninstall script in the install directory of the updater.
+// 1. deletes the scheduled task.
+// 2. runs the uninstall script in the install directory of the updater.
 // The execution of this function and the script race each other but the script
 // loops and waits in between iterations trying to delete the install directory.
 int Uninstall() {
@@ -49,16 +45,6 @@ int Uninstall() {
 
   updater::UnregisterUpdateAppsTask();
 
-  std::unique_ptr<WorkItemList> uninstall_list(WorkItem::CreateWorkItemList());
-  uninstall_list->AddDeleteRegKeyWorkItem(HKEY_CURRENT_USER,
-                                          base::ASCIIToUTF16(UPDATER_KEY),
-                                          WorkItem::kWow64Default);
-  if (!uninstall_list->Do()) {
-    LOG(ERROR) << "Failed to delete the registry keys.";
-    uninstall_list->Rollback();
-    return -1;
-  }
-
   base::FilePath product_dir;
   if (!GetProductDirectory(&product_dir)) {
     LOG(ERROR) << "GetProductDirectory failed.";
@@ -74,7 +60,8 @@ int Uninstall() {
   base::FilePath script_path = product_dir.AppendASCII(kUninstallScript);
 
   base::string16 cmdline = cmd_path;
-  base::StringAppendF(&cmdline, L" /Q /C \"%ls\"", script_path.value().c_str());
+  base::StringAppendF(&cmdline, L" /Q /C \"%ls\"",
+                      script_path.AsUTF16Unsafe().c_str());
   base::LaunchOptions options;
   options.start_hidden = true;
 

chrome/updater/win/util.cc

@@ -13,10 +13,8 @@
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/process/process_iterator.h"
-#include "base/strings/strcat.h"
 #include "base/strings/string16.h"
-#include "base/strings/utf_string_conversions.h"
-#include "chrome/updater/updater_constants.h"
+#include "base/strings/sys_string_conversions.h"
 #include "chrome/updater/win/constants.h"
 #include "chrome/updater/win/user_info.h"
 
@@ -170,7 +168,7 @@ HRESULT CreateUniqueEventInEnvironment(const base::string16& var_name,
                                        HANDLE* unique_event) {
   DCHECK(unique_event);
 
-  const base::string16 event_name = base::ASCIIToUTF16(base::GenerateGUID());
+  const base::string16 event_name = base::SysUTF8ToWide(base::GenerateGUID());
   NamedObjectAttributes attr;
   GetNamedObjectAttributes(event_name.c_str(), is_machine, &attr);
 
@@ -303,13 +301,4 @@ void GetAdminDaclSecurityAttributes(CSecurityAttributes* sec_attr,
   GetAdminDaclSecurityDescriptor(&sd, accessmask);
   sec_attr->Set(sd);
 }
-
-base::string16 GetRegistryKeyClientsUpdater() {
-  return base::ASCIIToUTF16(base::StrCat({CLIENTS_KEY, kUpdaterAppId}));
-}
-
-base::string16 GetRegistryKeyClientStateUpdater() {
-  return base::ASCIIToUTF16(base::StrCat({CLIENT_STATE_KEY, kUpdaterAppId}));
-}
-
 }  // namespace updater