Make the updater setup install all the updater's runtime dependencies.

This CL removes the hardcoded list of files in the updater's setup.
At runtime, the setup copies all files unpacked by the metainstaller, or
uses a build-generated dependency file to determine what files it needs
to install, if `updater --install` is run from the `out` directory of
the build.

The CL introduces a new command line argument `--install-from-out-dir`,
which is used in addition to other command line arguments of
`--install`.

This CL fixes the issue where --install could not find the
`uninstall.cmd` file when run from the `out` dir.

Bug:1069264,1054685

Change-Id: I03382b6d60fc71352a650e90b9abad6a85f085e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146082Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758416}

chrome/updater/constants.cc

@@ -27,6 +27,10 @@ const char kLoggingModuleSwitch[] = "vmodule";
 const char kSingleProcessSwitch[] = "single-process";
 const char kAppIdSwitch[] = "appid";
 
+#if defined(OS_WIN)
+const char kInstallFromOutDir[] = "install-from-out-dir";
+#endif  // OS_WIN
+
 // URLs.
 const char kUpdaterJSONDefaultUrl[] =
     "https://update.googleapis.com/service/update2/json";

chrome/updater/constants.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_UPDATER_CONSTANTS_H_
 #define CHROME_UPDATER_CONSTANTS_H_
 
+#include "build/build_config.h"
 #include "components/update_client/update_client_errors.h"
 
 namespace updater {
@@ -55,6 +56,14 @@ extern const char kCrashHandlerSwitch[];
 // Installs the updater.
 extern const char kInstallSwitch[];
 
+#if defined(OS_WIN)
+// A debug switch to indicate that --install is running from the `out` directory
+// of the build. When this switch is present, the setup picks up the run time
+// dependencies of the updater from the `out` directory instead of using the
+// metainstaller uncompressed archive.
+extern const char kInstallFromOutDir[];
+#endif  // OS_WIN
+
 // Uninstalls the updater.
 extern const char kUninstallSwitch[];
 

chrome/updater/tag.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/updater/tag.h"
 
+#include <map>
+#include <utility>
+
 #include "base/no_destructor.h"
 #include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
@@ -11,6 +14,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "chrome/updater/lib_util.h"
+#include "chrome/updater/util.h"
 
 namespace updater {
 namespace tagging {
@@ -104,15 +108,8 @@ base::Optional<bool> ParseBool(base::StringPiece str) {
   return base::nullopt;
 }
 
-// Functor for case-insensitive ASCII comparisons with StringPiece for std::map.
-//
-// See base::CaseInsensitiveCompareASCII for usage with unicode.
-struct CaseInsensitiveCompareASCII {
- public:
-  bool operator()(base::StringPiece x, base::StringPiece y) const {
-    return CompareCaseInsensitiveASCII(x, y) > 0;
-  }
-};
+// A custom comparator functor class for the parse tables.
+using ParseTableCompare = CaseInsensitiveASCIICompare<std::string>;
 
 namespace global_attributes {
 
@@ -217,9 +214,8 @@ ErrorCode ParseAppId(base::StringPiece value, TagArgs* args) {
 using ParseGlobalAttributeFunPtr = ErrorCode (*)(base::StringPiece value,
                                                  TagArgs* args);
 
-using GlobalParseTable = std::map<base::StringPiece,
-                                  ParseGlobalAttributeFunPtr,
-                                  CaseInsensitiveCompareASCII>;
+using GlobalParseTable =
+    std::map<base::StringPiece, ParseGlobalAttributeFunPtr, ParseTableCompare>;
 
 const GlobalParseTable& GetTable() {
   static const base::NoDestructor<GlobalParseTable> instance{{
@@ -289,9 +285,8 @@ ErrorCode ParseUntrustedData(base::StringPiece value, AppArgs* args) {
 using ParseAppAttributeFunPtr = ErrorCode (*)(base::StringPiece value,
                                               AppArgs* args);
 
-using AppParseTable = std::map<base::StringPiece,
-                               ParseAppAttributeFunPtr,
-                               CaseInsensitiveCompareASCII>;
+using AppParseTable =
+    std::map<base::StringPiece, ParseAppAttributeFunPtr, ParseTableCompare>;
 
 const AppParseTable& GetTable() {
   static const base::NoDestructor<AppParseTable> instance{{
@@ -355,7 +350,7 @@ using ParseInstallerDataAttributeFunPtr =
 
 using InstallerDataParseTable = std::map<base::StringPiece,
                                          ParseInstallerDataAttributeFunPtr,
-                                         CaseInsensitiveCompareASCII>;
+                                         ParseTableCompare>;
 
 const InstallerDataParseTable& GetTable() {
   static const base::NoDestructor<InstallerDataParseTable> instance{{

chrome/updater/test/data/updater.runtime_deps

+./updater.exe
+./updater.exe.pdb
+./base.dll
+./base.dll.pdb
+msvcp140d.dll
+gen/mojo/public/interfaces/bindings/interface_control_messages.mojom.js
+icudtl.dat
+gen/chrome/updater/win/uninstall.cmd

chrome/updater/updater.cc

@@ -40,6 +40,9 @@
 // directory of the build.
 // - To debug, append the following arguments to any updater command line:
 //    --enable-logging --vmodule=*/chrome/updater/*=2.
+// - To run the `updater --install` from the `out` directory of the build,
+//   use --install-from-out-dir command line switch in addition to other
+//   arguments for --install.
 
 namespace updater {
 

chrome/updater/util.h

@@ -6,6 +6,8 @@
 #define CHROME_UPDATER_UTIL_H_
 
 #include "base/files/file_path.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
 
 namespace updater {
 
@@ -15,6 +17,17 @@ bool GetProductDirectory(base::FilePath* path);
 // Initializes logging for an executable.
 void InitLogging(const base::FilePath::StringType& filename);
 
+// Functor used by associative containers of strings as a case-insensitive ASCII
+// compare. |T| could be std::string or base::string16.
+template <typename T>
+struct CaseInsensitiveASCIICompare {
+ public:
+  bool operator()(base::BasicStringPiece<T> x,
+                  base::BasicStringPiece<T> y) const {
+    return base::CompareCaseInsensitiveASCII(x, y) > 0;
+  }
+};
+
 }  // namespace updater
 
 #endif  // CHROME_UPDATER_UTIL_H_

chrome/updater/win/BUILD.gn

@@ -29,6 +29,8 @@ executable("updater") {
     "//chrome/updater:lib",
     "//chrome/updater/win/ui:ui_resources",
   ]
+
+  data_deps = [ ":uninstall.cmd" ]
 }
 
 copy("uninstall.cmd") {
@@ -184,6 +186,7 @@ test("updater_unittests") {
 
   sources = [
     "//chrome/updater/win/test/test_main.cc",
+    "setup/setup_util_unittest.cc",
     "task_scheduler_unittest.cc",
   ]
 
@@ -198,4 +201,6 @@ test("updater_unittests") {
     "//chrome/updater/win/test:test_strings",
     "//testing/gtest",
   ]
+
+  data = [ "//chrome/updater/test/data/updater.runtime_deps" ]
 }

chrome/updater/win/installer/BUILD.gn

@@ -38,7 +38,7 @@ process_version_rc_template("version") {
 # This target creats a list of runtime dependencies for the component
 # builds. This list is parsed by the |create_installer_archive| script, the
 # DLL paths extracted out from the list, and included in the archive.
-updater_runtime_deps = "$root_gen_dir/updater.runtime_deps"
+updater_runtime_deps = "$target_gen_dir/updater.runtime_deps"
 group("updater_runtime_deps") {
   write_runtime_deps = updater_runtime_deps
   data_deps = [ "//chrome/updater/win:updater" ]
@@ -83,7 +83,6 @@ template("generate_installer") {
 
     deps = [
       ":updater_runtime_deps",
-      "//chrome/updater/win:uninstall.cmd",
       "//chrome/updater/win:updater",
       "//third_party/icu:icudata",
     ]

chrome/updater/win/setup/setup.cc

@@ -7,11 +7,13 @@
 #include <shlobj.h>
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
@@ -34,30 +36,8 @@
 #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",
-    L"protobuf_lite.dll",
-    L"url_lib.dll",
-    L"zlib.dll",
-#endif
-};
-
 // Adds work items to register the COM Server with Windows.
 void AddComServerWorkItems(HKEY root,
                            const base::FilePath& com_server_path,
@@ -183,8 +163,38 @@ void AddComInterfacesWorkItems(HKEY root,
   }
 }
 
+// Returns a list of base file names which the setup copies to the install
+// directory. The source of these files is either the unpacked metainstaller
+// archive, or the `out` directory of the build, if a command line argument is
+// present. In the former case, which is the normal execution flow, the files
+// are enumerated from the directory where the metainstaller unpacked its
+// contents. In the latter case, the file containing the run time dependencies
+// of the updater (which is generated by GN at build time) is parsed, and the
+// relevant file names are extracted.
+std::vector<base::FilePath> GetSetupFiles(const base::FilePath& source_dir) {
+  const auto* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(kInstallFromOutDir)) {
+    return ParseFilesFromDeps(source_dir.Append(FILE_PATH_LITERAL(
+        "gen\\chrome\\updater\\win\\installer\\updater.runtime_deps")));
+  }
+  std::vector<base::FilePath> result;
+  base::FileEnumerator it(
+      source_dir, false, base::FileEnumerator::FileType::FILES,
+      FILE_PATH_LITERAL("*"), base::FileEnumerator::FolderSearchPolicy::ALL,
+      base::FileEnumerator::ErrorPolicy::STOP_ENUMERATION);
+  for (base::FilePath file = it.Next(); !file.empty(); file = it.Next()) {
+    result.push_back(file.BaseName());
+  }
+  if (it.GetError() != base::File::Error::FILE_OK) {
+    VLOG(2) << __func__ << " could not enumerate files : " << it.GetError();
+    return {};
+  }
+  return result;
+}
+
 }  // namespace
 
+// TODO(crbug.com/1069976): use specific return values for different code paths.
 int Setup(bool is_machine) {
   VLOG(1) << __func__ << ", is_machine: " << is_machine;
   DCHECK(!is_machine || ::IsUserAnAdmin());
@@ -216,11 +226,18 @@ int Setup(bool is_machine) {
     return -1;
   }
 
-  const base::FilePath source_dir = exe_path.DirName();
+  const auto source_dir = exe_path.DirName();
+  const auto setup_files = GetSetupFiles(source_dir);
+  if (setup_files.empty()) {
+    LOG(ERROR) << "No files to set up.";
+    return -1;
+  }
 
+  // All source files are installed in a flat directory structure inside the
+  // install directory, hence the BaseName function call below.
   std::unique_ptr<WorkItemList> install_list(WorkItem::CreateWorkItemList());
-  for (const auto* file : kUpdaterFiles) {
-    const base::FilePath target_path = product_dir.Append(file);
+  for (const auto& file : setup_files) {
+    const base::FilePath target_path = product_dir.Append(file.BaseName());
     const base::FilePath source_path = source_dir.Append(file);
     install_list->AddCopyTreeWorkItem(source_path.value(), target_path.value(),
                                       temp_dir.value(), WorkItem::ALWAYS);

chrome/updater/win/setup/setup_util.cc

@@ -4,14 +4,22 @@
 
 #include "chrome/updater/win/setup/setup_util.h"
 
+#include <string>
+
 #include "base/command_line.h"
+#include "base/containers/flat_set.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_split.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/win/win_util.h"
-#include "chrome/updater/win/task_scheduler.h"
 #include "chrome/updater/server/win/updater_idl.h"
+#include "chrome/updater/util.h"
+#include "chrome/updater/win/task_scheduler.h"
 
 namespace updater {
 
@@ -66,4 +74,26 @@ base::string16 GetComTypeLibRegistryPath(REFIID iid) {
       {L"Software\\Classes\\TypeLib\\", base::win::String16FromGUID(iid)});
 }
 
+std::vector<base::FilePath> ParseFilesFromDeps(const base::FilePath& deps) {
+  constexpr size_t kDepsFileSizeMax = 0x2000;  // 8KB.
+  std::string contents;
+  if (!base::ReadFileToStringWithMaxSize(deps, &contents, kDepsFileSizeMax))
+    return {};
+  const base::flat_set<const base::char16*,
+                       CaseInsensitiveASCIICompare<base::string16>>
+      exclude_extensions = {L".pdb", L".js"};
+  std::vector<base::FilePath> result;
+  for (const auto& line :
+       base::SplitString(contents, "\r\n", base::TRIM_WHITESPACE,
+                         base::SPLIT_WANT_NONEMPTY)) {
+    const auto filename =
+        base::FilePath(base::ASCIIToUTF16(line)).NormalizePathSeparators();
+    if (!base::Contains(exclude_extensions,
+                        filename.FinalExtension().c_str())) {
+      result.push_back(filename);
+    }
+  }
+  return result;
+}
+
 }  // namespace updater

chrome/updater/win/setup/setup_util.h

@@ -7,13 +7,16 @@
 
 #include <guiddef.h>
 
-namespace base {
-class CommandLine;
-}  // namespace base
+#include <vector>
 
 #include "base/strings/string16.h"
 #include "base/win/windows_types.h"
 
+namespace base {
+class CommandLine;
+class FilePath;
+}  // namespace base
+
 namespace updater {
 
 bool RegisterUpdateAppsTask(const base::CommandLine& run_command);
@@ -26,6 +29,12 @@ base::string16 GetComServiceAppidRegistryPath();
 base::string16 GetComIidRegistryPath(REFIID iid);
 base::string16 GetComTypeLibRegistryPath(REFIID iid);
 
+// Parses the run time dependency file which contains all dependencies of
+// the `updater` target. This file is a text file, where each line of
+// text represents a single dependency. Some dependencies are not needed for
+// updater to run, and are filtered out from the return value of this function.
+std::vector<base::FilePath> ParseFilesFromDeps(const base::FilePath& deps);
+
 }  // namespace updater
 
 #endif  // CHROME_UPDATER_WIN_SETUP_SETUP_UTIL_H_

chrome/updater/win/setup/setup_util_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/updater/win/setup/setup_util.h"
+#include "base/base_paths.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace updater {
+
+TEST(UpdaterSetupUtil, ParseFilesFromDeps) {
+  base::FilePath source_path;
+  ASSERT_TRUE(base::PathService::Get(base::DIR_SOURCE_ROOT, &source_path));
+  const base::FilePath deps = source_path.AppendASCII("chrome")
+                                  .AppendASCII("updater")
+                                  .AppendASCII("test")
+                                  .AppendASCII("data")
+                                  .AppendASCII("updater.runtime_deps");
+  const auto files = ParseFilesFromDeps(deps);
+  EXPECT_EQ(files.size(), 5u);
+  EXPECT_EQ(files[0], base::FilePath(FILE_PATH_LITERAL(".\\updater.exe")));
+  EXPECT_EQ(files[1], base::FilePath(FILE_PATH_LITERAL(".\\base.dll")));
+  EXPECT_EQ(files[2], base::FilePath(FILE_PATH_LITERAL("msvcp140d.dll")));
+  EXPECT_EQ(files[3], base::FilePath(FILE_PATH_LITERAL("icudtl.dat")));
+  EXPECT_EQ(files[4], base::FilePath(FILE_PATH_LITERAL(
+                          "gen\\chrome\\updater\\win\\uninstall.cmd")));
+}
+
+}  // namespace updater