Replace early check for PNaCl with an on-demand check.

chrome/browser/component_updater/pnacl/pnacl_component_installer.cc

 // Copyright (c) 2012 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/pnacl/pnacl_component_installer.h"
 
 #include "base/base_paths.h"
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "base/win/windows_version.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/component_updater/component_updater_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/omaha_query_params/omaha_query_params.h"
 #include "content/public/browser/browser_thread.h"
 
 using chrome::OmahaQueryParams;
 using content::BrowserThread;
 
 namespace {
 
-// If PNaCl isn't installed yet, but a user is running chrome with
-// --enable-pnacl, this is the amount of time to wait before starting
-// a background install.
-const int kInitialDelaySeconds = 10;
+// Time in seconds to wait for CheckUpdatesForPnacl to complete
+// before considering the update failed.
+int kPnaclInstallerTimeout = 60;
 
 // Name of the Pnacl component specified in the manifest.
 const char kPnaclManifestNamePrefix[] = "PNaCl";
 
-// Sanitize characters from Pnacl Arch value so that they can be used
-// in path names.  This should only be characters in the set: [a-z0-9_].
-// Keep in sync with chrome/browser/nacl_host/nacl_file_host.
-std::string SanitizeForPath(const std::string& input) {
-  std::string result;
-  ReplaceChars(input, "-", "_", &result);
-  return result;
-}
-
 // Set the component's hash to the arch-specific PNaCl package.
 void SetPnaclHash(CrxComponent* component) {
 #if defined(ARCH_CPU_X86_FAMILY)
   // Define both x86_32 and x86_64, and choose below.
   static const uint8 x86_sha256_hash[][32] = {
     { // This corresponds to AppID (x86-32): aealhdcgieaiikaifafholmmeooeeioj
       0x04, 0x0b, 0x73, 0x26, 0x84, 0x08, 0x8a, 0x08, 0x50, 0x57,
       0xeb, 0xcc, 0x4e, 0xe4, 0x48, 0xe9, 0x44, 0x2c, 0xc8, 0xa6, 0xd6,
       0x96, 0x11, 0xd4, 0x2a, 0xc5, 0x26, 0x64, 0x34, 0x76, 0x3d, 0x14},
     { // This corresponds to AppID (x86-64): knlfebnofcjjnkpkapbgfphaagefndik
@@ skipping 149 matching lines @@
                  << arch << " vs " << OmahaQueryParams::getNaclArch() << ")";
     return false;
   }
 
   *version_out = version;
   return true;
 }
 
 PnaclComponentInstaller::PnaclComponentInstaller()
     : per_user_(false),
-      cus_(NULL) {
+      cus_(NULL),
+      callback_nums_(0) {
 #if defined(OS_CHROMEOS)
   per_user_ = true;
 #endif
 }
 
 PnaclComponentInstaller::~PnaclComponentInstaller() {
 }
 
 void PnaclComponentInstaller::OnUpdateError(int error) {
   NOTREACHED() << "Pnacl update error: " << error;
@@ skipping 26 matching lines @@
   // On chromeos, we want to find the --login-profile=<foo> dir.
   // Even though the path does vary between users, the content
   // changes when logging out and logging in.
   ProfileManager* pm = g_browser_process->profile_manager();
   current_profile_path_ = pm->user_data_dir().Append(
       pm->GetInitialProfileDir());
 }
 
 bool PnaclComponentInstaller::Install(const base::DictionaryValue& manifest,
                                       const base::FilePath& unpack_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   scoped_ptr<base::DictionaryValue> pnacl_manifest(
       ReadPnaclManifest(unpack_path));
   if (pnacl_manifest == NULL) {
     LOG(WARNING) << "Failed to read pnacl manifest.";
+    NotifyInstallError();
     return false;
   }
 
   Version version;
   if (!CheckPnaclComponentManifest(manifest, *pnacl_manifest, &version)) {
     LOG(WARNING) << "CheckPnaclComponentManifest failed, not installing.";
+    NotifyInstallError();
     return false;
   }
 
   // Don't install if the current version is actually newer.
-  if (current_version().CompareTo(version) > 0)
+  if (current_version().CompareTo(version) > 0) {
+    NotifyInstallError();
     return false;
+  }
 
   // Passed the basic tests. Time to install it.
   base::FilePath path = GetPnaclBaseDirectory().AppendASCII(
       version.GetString());
   if (file_util::PathExists(path)) {
     LOG(WARNING) << "Target path already exists, not installing.";
+    NotifyInstallError();
     return false;
   }
   if (!file_util::Move(unpack_path, path)) {
     LOG(WARNING) << "Move failed, not installing.";
+    NotifyInstallError();
     return false;
   }
 
-  // Installation is done. Now tell the rest of chrome (just the path service
-  // for now). TODO(jvoung): we need notifications if someone surfed to a
-  // Pnacl webpage and Pnacl was just installed at this time. They should
-  // then be able to reload the page and retry (or something).
-  // See: http://code.google.com/p/chromium/issues/detail?id=107438
+  // Installation is done. Now tell the rest of chrome.
+  // - The path service.
+  // - Callbacks that requested an update.
   set_current_version(version);
-
+  NotifyInstallSuccess();
   PathService::Override(chrome::DIR_PNACL_COMPONENT, path);
   return true;
 }
 
-namespace {
+void PnaclComponentInstaller::AddInstallCallback(
+    const InstallCallback& cb) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  int num = ++callback_nums_;
+  install_callbacks_.push_back(std::make_pair(cb, num));
 
-void DoCheckForUpdate(ComponentUpdateService* cus,
-                   const CrxComponent& pnacl) {
-  if (cus->CheckForUpdateSoon(pnacl) != ComponentUpdateService::kOk) {
-    LOG(WARNING) << "Pnacl check for update failed.";
+  // Set a timeout.  If the install doesn't complete within a minute,
+  // assume that the update failed and cancel the callback.
+  // Do this on the same thread that would have checked the callbacks.
+  BrowserThread::PostDelayedTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&PnaclComponentInstaller::CancelCallback,
+                 // Why unretained? The installer should have

[sehr, 2013/06/14 18:23:43]

Perhaps a better phrasing would be "we use Unretained here because the installer
...".

The other way makes it sound like *you* don't know why you used it.
Or was that what you intended?

[jvoung (off chromium), 2013/06/14 18:57:50]

Done.

+                 // the same lifetime as the component updater service,
+                 // which lives until process shutdown.
+                 base::Unretained(this),
+                 num),
+      base::TimeDelta::FromSeconds(kPnaclInstallerTimeout));
+}
+
+void PnaclComponentInstaller::CancelCallback(int num) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  for (std::list<std::pair<InstallCallback, int> >::iterator
+           i = install_callbacks_.begin(),
+           e = install_callbacks_.end(); i != e; ++i) {
+    if (i->second == num) {
+      i->first.Run(false);
+      install_callbacks_.erase(i);
+      return;
+    }
   }
 }
 
-// Finally, do the registration with the right version number.
+void PnaclComponentInstaller::NotifyAllWithResult(bool status) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  while (!install_callbacks_.empty()) {
+    install_callbacks_.front().first.Run(status);
+    install_callbacks_.pop_front();
+  }
+}
+
+void PnaclComponentInstaller::NotifyInstallError() {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&PnaclComponentInstaller::NotifyAllWithResult,
+                 // Why unretained? The installer lives until process shutdown.

[sehr, 2013/06/14 18:23:43]

Same comment issue here.

[jvoung (off chromium), 2013/06/14 18:57:50]

Done.

+                 base::Unretained(this), false));
+}
+
+
+void PnaclComponentInstaller::NotifyInstallSuccess() {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&PnaclComponentInstaller::NotifyAllWithResult,
+                 // Why unretained? The installer lives until process shutdown.
+                 base::Unretained(this), true));
+}
+
+namespace {
+
 void FinishPnaclUpdateRegistration(const Version& current_version,
                                    PnaclComponentInstaller* pci) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   CrxComponent pnacl_component;
   pnacl_component.version = current_version;
   pnacl_component.name = "pnacl";
   pnacl_component.installer = pci;
   pci->set_current_version(current_version);
   SetPnaclHash(&pnacl_component);
 
   ComponentUpdateService::Status status =
       pci->cus()->RegisterComponent(pnacl_component);
   if (status != ComponentUpdateService::kOk
       && status != ComponentUpdateService::kReplaced) {
     NOTREACHED() << "Pnacl component registration failed.";
   }
-
-  // If PNaCl is not yet installed but it is requested by --enable-pnacl,
-  // we want it to be available "soon", so kick off an update check
-  // earlier than usual.
-  Version null_version(kNullVersion);
-  if (pci->current_version().Equals(null_version)) {
-    BrowserThread::PostDelayedTask(
-        BrowserThread::UI, FROM_HERE,
-        base::Bind(DoCheckForUpdate, pci->cus(), pnacl_component),
-        base::TimeDelta::FromSeconds(kInitialDelaySeconds));
-  }
 }
 
 // Check if there is an existing version on disk first to know when
 // a hosted version is actually newer.
 void StartPnaclUpdateRegistration(PnaclComponentInstaller* pci) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   base::FilePath path = pci->GetPnaclBaseDirectory();
   if (!file_util::PathExists(path)) {
     if (!file_util::CreateDirectory(path)) {
       NOTREACHED() << "Could not create base Pnacl directory.";
@@ skipping 44 matching lines @@
      BrowserThread::FILE, FROM_HERE,
      base::Bind(&StartPnaclUpdateRegistration, pci));
 }
 
 
 }  // namespace
 
 void PnaclComponentInstaller::RegisterPnaclComponent(
                             ComponentUpdateService* cus,
                             const CommandLine& command_line) {
-  // Only register when given the right flag.  This is important since
-  // we do an early component updater check above (in DoCheckForUpdate).
+  // Only register when given the right flag, for now.
   if (command_line.HasSwitch(switches::kEnablePnacl)) {
     cus_ = cus;
     // If per_user, create a profile observer to watch for logins.
     // Only do so after cus_ is set to something non-null.
     if (per_user_ && !profile_observer_) {
       profile_observer_.reset(new PnaclProfileObserver(this));
     }
     if (per_user_) {
       // Figure out profile information, before proceeding to look for files.
-      BrowserThread::PostTask(
-           BrowserThread::UI, FROM_HERE,
-           base::Bind(&GetProfileInformation, this));
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                              base::Bind(&GetProfileInformation, this));
     } else {
-      BrowserThread::PostTask(
-           BrowserThread::FILE, FROM_HERE,
-           base::Bind(&StartPnaclUpdateRegistration, this));
+      BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                              base::Bind(&StartPnaclUpdateRegistration, this));
     }
   }
 }
 
 void PnaclComponentInstaller::ReRegisterPnacl() {
   // No need to check the commandline flags again here.
   // We could only have gotten here after RegisterPnaclComponent
   // found --enable-pnacl, since that is where we create the profile_observer_,
   // which in turn calls ReRegisterPnacl.
   DCHECK(per_user_);
   // Figure out profile information, before proceeding to look for files.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&GetProfileInformation, this));
 }
+
+void RequestFirstInstall(ComponentUpdateService* cus,
+                         PnaclComponentInstaller* pci,
+                          const base::Callback<void(bool)>& installed) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  Version null_version(kNullVersion);
+  CrxComponent pnacl_component;
+  pci->set_current_version(null_version);
+  pnacl_component.version = null_version;
+  pnacl_component.name = "pnacl";
+  pnacl_component.installer = pci;
+  SetPnaclHash(&pnacl_component);
+  ComponentUpdateService::Status status = cus->CheckForUpdateSoon(
+      pnacl_component);
+  if (status != ComponentUpdateService::kOk) {
+    installed.Run(false);
+    return;
+  }
+  pci->AddInstallCallback(installed);
+}