Adding a SW reporter component updater

chrome/browser/component_updater/sw_reporter_installer_win.cc

+// Copyright (c) 2014 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/browser/component_updater/sw_reporter_installer_win.h"
+
+#include <string>
+#include <vector>
+
+#include "base/base_paths.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
+#include "base/path_service.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/pref_service.h"
+#include "base/process/kill.h"
+#include "base/process/launch.h"
+#include "base/threading/worker_pool.h"
+#include "base/win/registry.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/component_updater/component_updater_service.h"
+#include "chrome/browser/component_updater/default_component_installer.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace component_updater {
+
+namespace {
+
+// These values are used to send UMA information and are replicated in the
+// histograms.xml file, so the order MUST NOT CHANGE.
+enum SwReporterUmaValue {
+  SW_REPORTER_EXPLICIT_REQUEST = 0,
+  SW_REPORTER_STARTUP_RETRY = 1,
+  SW_REPORTER_RETRIED_TOO_MANY_TIMES = 2,
+  SW_REPORTER_START_EXECUTION = 3,
+  SW_REPORTER_FAILED_TO_START = 4,
+  SW_REPORTER_UPDATE_WITHOUT_REQUEST = 5,
+  SW_REPORTER_REGISTRY_EXIT_CODE = 6,
+  SW_REPORTER_MAX,
+};
+
+// The maximum number of times to retry a download on startup.
+const int kMaxRetry = 7;
+
+// CRX hash. The extension id is: gkmgaooipdjhmangpemjhigmamcehddo. The hash was
+// generated in Python with something like this:
+// hashlib.sha256().update(open("<file>.crx").read()[16:16+294]).digest().
+const uint8 kSha256Hash[] = {0x6a, 0xc6, 0x0e, 0xe8, 0xf3, 0x97, 0xc0, 0xd6,
+                             0xf4, 0xc9, 0x78, 0x6c, 0x0c, 0x24, 0x73, 0x3e,
+                             0x05, 0xa5, 0x62, 0x4b, 0x2e, 0xc7, 0xb7, 0x1c,
+                             0x5f, 0xea, 0xf0, 0x88, 0xf6, 0x97, 0x9b, 0xc7};
+
+const base::FilePath::CharType kSwReporterExeName[] =
+    FILE_PATH_LITERAL("software_reporter_tool.exe");
+
+// Where to fetch the reporter exit code in the registry.
+const wchar_t kSoftwareRemovalToolRegistryKey[] =
+    L"Software\\Google\\Software Removal Tool";
+const wchar_t kExitCodeRegistryValueName[] = L"ExitCode";
+
+void ReportUmaStep(SwReporterUmaValue value) {
+  UMA_HISTOGRAM_ENUMERATION("SoftwareReporter.Step", value, SW_REPORTER_MAX);
+}
+
+// This function is called on the UI thread to report the SwReporter exit code
+// and then clear it from the registry as well as clear the execution state
+// from the local state. This could be called from an interruptible worker
+// thread so should be resilient to unexpected shutdown.
+void ReportAndClearExitCode(int exit_code) {
+  UMA_HISTOGRAM_SPARSE_SLOWLY("SoftwareReporter.ExitCode", exit_code);
+
+  base::win::RegKey srt_key(
+      HKEY_CURRENT_USER, kSoftwareRemovalToolRegistryKey, KEY_WRITE);
+  srt_key.DeleteValue(kExitCodeRegistryValueName);
+
+  // Now that we are done we can reset the try count.
+  g_browser_process->local_state()->SetInteger(
+      prefs::kSwReporterExecuteTryCount, 0);
+}
+
+// This function is called from a worker thread to launch the SwReporter and
+// wait for termination to collect its exit code. This task could be interrupted
+// by a shutdown at anytime, so it shouldn't depend on anything external that
+// could be shutdown beforehand.
+void LaunchAndWaitForExit(const base::FilePath& exe_path) {
+  const base::CommandLine reporter_command_line(exe_path);
+  base::ProcessHandle scan_reporter_process = base::kNullProcessHandle;
+  if (!base::LaunchProcess(reporter_command_line,
+                           base::LaunchOptions(),
+                           &scan_reporter_process)) {
+    ReportUmaStep(SW_REPORTER_FAILED_TO_START);
+    return;
+  }
+  ReportUmaStep(SW_REPORTER_START_EXECUTION);
+
+  int exit_code = -1;
+  bool success = base::WaitForExitCode(scan_reporter_process, &exit_code);
+  DCHECK(success);
+  base::CloseProcessHandle(scan_reporter_process);
+  scan_reporter_process = NULL;

[Sorin Jianu, 2014/06/20 00:31:43]

base::kNullProcessHandle

Is there any need to reset the handle anyway?

[MAD, 2014/06/20 04:51:59]

Done.
I always prefer not keeping an invalid value in a variable. Since the handle has
been closed, it shouldn't be kept.

+  // It's OK if this doesn't complete, the work will continue on next startup.
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          base::Bind(&ReportAndClearExitCode, exit_code));
+}
+
+void ExecuteReporter(const base::FilePath& install_dir) {
+  base::WorkerPool::PostTask(
+      FROM_HERE,
+      base::Bind(&LaunchAndWaitForExit, install_dir.Append(kSwReporterExeName)),
+      true);
+}
+
+class SwReporterComponentInstallerTraits : public ComponentInstallerTraits {
+ public:
+  explicit SwReporterComponentInstallerTraits(PrefService* prefs)
+      : prefs_(prefs) {
+    // We might get called to be registered again if files were manually deleted
+    // so make sure to start from scratch.
+    install_dir_.clear();
+  }
+
+  virtual ~SwReporterComponentInstallerTraits() {}
+
+  virtual bool VerifyInstallation(const base::FilePath& dir) const {
+    return base::PathExists(dir.Append(kSwReporterExeName));
+  }
+
+  virtual bool CanAutoUpdate() const { return true; }
+
+  virtual bool OnCustomInstall(const base::DictionaryValue& manifest,
+                               const base::FilePath& install_dir) {
+    return true;
+  }
+
+  virtual void ComponentReady(const base::Version& version,
+                              const base::FilePath& install_dir,
+                              scoped_ptr<base::DictionaryValue> manifest) {
+    // Remember the path, in case we get called again.

[Sorin Jianu, 2014/06/20 00:31:43]

Comment is redundant. We would need specific comments to explain in detail how
the whole design works. But this comment is not needed here.

[MAD, 2014/06/20 04:51:59]

Done.

+    install_dir_ = install_dir;
+    // If we get here without an execute try count, this means that the
+    // component have been silently updated to a new version, so we don't want
+    // to execute.
+    if (prefs_->GetInteger(prefs::kSwReporterExecuteTryCount) == 0) {
+      ReportUmaStep(SW_REPORTER_UPDATE_WITHOUT_REQUEST);
+      return;
+    }
+
+    // Now execute the reporter from a worker thread.
+    ExecuteReporter(install_dir);
+  }
+
+  virtual base::FilePath GetBaseDirectory() const {
+    // The base directory on windows looks like:
+    // <profile>\AppData\Local\Google\Chrome\User Data\SwReporter\.
+    base::FilePath result;
+    PathService::Get(chrome::DIR_USER_DATA, &result);
+    return result.Append(FILE_PATH_LITERAL("SwReporter"));
+  }
+
+  virtual void GetHash(std::vector<uint8>* hash) const {
+    DCHECK(hash);
+    hash->assign(kSha256Hash, &kSha256Hash[sizeof(kSha256Hash)]);

[Sorin Jianu, 2014/06/20 00:31:43]

small nit, I would write something to better see the first and last iterators,
like:
hash->assign(kSha256Hash, kSha256Hash + sizeof(kSha256Hash));

[MAD, 2014/06/20 04:51:59]

Done.

+  }
+
+  virtual std::string GetName() const { return ID(); }
+
+  static const base::FilePath& install_dir() { return install_dir_; }
+  static const char* ID() { return "Software Reporter Tool"; }

[Sorin Jianu, 2014/06/20 00:31:43]

This could be a bug I believe, since we should be searching for CRX ids, not
some human readable string. We could use component_updater::GetCrxComponentID to
give us such a string.

[MAD, 2014/06/20 04:51:59]

Done.

+
+ private:
+  PrefService* prefs_;
+  static base::FilePath install_dir_;

[Sorin Jianu, 2014/06/20 00:31:43]

static members of user defined types are forbidden by the coding style:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Static...

[MAD, 2014/06/20 04:51:59]

Done.

+};
+
+base::FilePath SwReporterComponentInstallerTraits::install_dir_;
+
+void RegisterComponent(ComponentUpdateService* cus, PrefService* prefs) {
+  scoped_ptr<ComponentInstallerTraits> traits(
+      new SwReporterComponentInstallerTraits(prefs));
+  // |cus| will take ownership of |installer| during installer->Register(cus).
+  DefaultComponentInstaller* installer =
+      new DefaultComponentInstaller(traits.Pass());
+  installer->Register(cus);
+}
+
+void RegisterOrExecuteReporter(ComponentUpdateService* cus,
+                               PrefService* prefs) {
+  if (!SwReporterComponentInstallerTraits::install_dir().empty())
+    ExecuteReporter(SwReporterComponentInstallerTraits::install_dir());
+  std::vector<std::string> registered_components(cus->GetComponentIDs());

[Sorin Jianu, 2014/06/20 00:31:43]

const

[MAD, 2014/06/20 04:51:59]

Done.

+  if (std::find(registered_components.begin(),
+                registered_components.end(),
+                SwReporterComponentInstallerTraits::ID()) ==

[Sorin Jianu, 2014/06/20 00:31:43]

See the ID comment above.

[MAD, 2014/06/20 04:51:59]

Done.

+      registered_components.end()) {
+    RegisterComponent(cus, prefs);
+  }
+}
+
+}  // namespace
+
+void ExecuteSwReporter(ComponentUpdateService* cus, PrefService* prefs) {
+  // This is an explicit call, so let's forget about previous incomplete
+  // execution attempts and start from scratch.
+  prefs->SetInteger(prefs::kSwReporterExecuteTryCount, 1);
+  ReportUmaStep(SW_REPORTER_EXPLICIT_REQUEST);
+  RegisterOrExecuteReporter(cus, prefs);
+}
+
+void ExecutePendingSwReporter(ComponentUpdateService* cus, PrefService* prefs) {
+  // Only continue registration / execute if we have a pending execution.
+  int execute_try_count = prefs->GetInteger(prefs::kSwReporterExecuteTryCount);
+  if (execute_try_count == 0)
+    return;
+  // We have a pending execution, let's check if it completed or not.
+  base::win::RegKey srt_key(
+      HKEY_CURRENT_USER, kSoftwareRemovalToolRegistryKey, KEY_READ);
+  DWORD exit_code = -1;
+  if (srt_key.Valid() &&
+      srt_key.ReadValueDW(kExitCodeRegistryValueName, &exit_code) ==
+          ERROR_SUCCESS) {
+    ReportUmaStep(SW_REPORTER_REGISTRY_EXIT_CODE);
+    ReportAndClearExitCode(exit_code);
+    return;
+  }
+
+  // If it didn't complete, let's make sure we didn't go beyond the maximum
+  // retry count yet.
+  if (++execute_try_count > kMaxRetry) {
+    ReportUmaStep(SW_REPORTER_RETRIED_TOO_MANY_TIMES);
+    // Let's forget about this attempt.
+    prefs->SetInteger(prefs::kSwReporterExecuteTryCount, 0);
+    return;
+  }
+  prefs->SetInteger(prefs::kSwReporterExecuteTryCount, execute_try_count);
+  ReportUmaStep(SW_REPORTER_STARTUP_RETRY);
+  RegisterOrExecuteReporter(cus, prefs);

[Sorin Jianu, 2014/06/20 00:31:43]

We must register for update at startup if we have traces of the component. All
code we ship must be able to auto-update.

There are many ways to resolve this. One idea would be that in addition to the
retry count in prefs, we maintain a bool that we've been invoked by the user. Or
we have a retry count that decrements from max-tries to 0.

The prefs state is check when we get called in the ComponentReady: either we
have to do something in response to a user action. Or it was just a silent
update, which we ignore, since the user did not ask us to run anything.

[MAD, 2014/06/20 04:51:59]

Done.

+}
+
+void RegisterPrefsForSwReporter(PrefRegistrySimple* registry) {
+  registry->RegisterIntegerPref(prefs::kSwReporterExecuteTryCount, 0);
+}
+
+}  // namespace component_updater