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

Only do this if PNaCl is not installed, or if the
current version is not compatible (for the wrong
architecture).

That way, when we decouple PNaCl component registration
from the --enable-pnacl flag, we won't have everybody doing
an early check when chrome starts.

Actual UI is still to be done/decided (so it's not pretty).
Furthermore, the component updater service delays each
step by N seconds, even during a CheckForUpdateSoon(),
so on a fast connection, most of the delay is the step timer.

BUG=107438
R=cpu@chromium.org, sehr@chromium.org

Review URL: https://codereview.chromium.org/17001003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207954 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/component_updater/pnacl/pnacl_component_installer.cc

@@ -6,6 +6,7 @@
 
 #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"
@@ -33,11 +34,6 @@ 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;
-
 // Name of the Pnacl component specified in the manifest.
 const char kPnaclManifestName[] = "PNaCl Translator";
 
@@ -194,10 +190,12 @@ bool CheckPnaclComponentManifest(const base::DictionaryValue& manifest,
 
 PnaclComponentInstaller::PnaclComponentInstaller()
     : per_user_(false),
-      cus_(NULL) {
+      cus_(NULL),
+      callback_nums_(0) {
 #if defined(OS_CHROMEOS)
   per_user_ = true;
 #endif
+  updater_observer_.reset(new PnaclUpdaterObserver(this));
 }
 
 PnaclComponentInstaller::~PnaclComponentInstaller() {
@@ -241,42 +239,47 @@ void PnaclComponentInstaller::OnProfileChange() {
 
 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();
   OverrideDirPnaclComponent(path);
   return true;
 }
@@ -286,16 +289,56 @@ bool PnaclComponentInstaller::GetInstalledFile(
   return false;
 }
 
-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 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;
+    }
+  }
+}
+
+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() {
+  if (!install_callbacks_.empty()) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&PnaclComponentInstaller::NotifyAllWithResult,
+                   // Unretained because installer lives until process shutdown.
+                   base::Unretained(this), false));
+  }
+}
 
-void DoCheckForUpdate(ComponentUpdateService* cus,
-                   const CrxComponent& pnacl) {
-  if (cus->CheckForUpdateSoon(pnacl) != ComponentUpdateService::kOk) {
-    LOG(WARNING) << "Pnacl check for update failed.";
+void PnaclComponentInstaller::NotifyInstallSuccess() {
+  if (!install_callbacks_.empty()) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&PnaclComponentInstaller::NotifyAllWithResult,
+                   // Unretained because installer lives until process shutdown.
+                   base::Unretained(this), true));
   }
 }
 
-// Finally, do the registration with the right version number.
+namespace {
+
 void FinishPnaclUpdateRegistration(const Version& current_version,
                                    PnaclComponentInstaller* pci) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -312,17 +355,6 @@ void FinishPnaclUpdateRegistration(const Version& current_version,
       && 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
@@ -389,8 +421,7 @@ void GetProfileInformation(PnaclComponentInstaller* pci) {
 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.
@@ -400,13 +431,11 @@ void PnaclComponentInstaller::RegisterPnaclComponent(
     }
     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));
     }
   }
 }
@@ -422,3 +451,23 @@ void PnaclComponentInstaller::ReRegisterPnacl() {
       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);
+}

chrome/browser/component_updater/pnacl/pnacl_component_installer.h

@@ -5,14 +5,16 @@
 #ifndef CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_COMPONENT_INSTALLER_H_
 #define CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_COMPONENT_INSTALLER_H_
 
-#include <string>
+#include <list>
 
+#include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/version.h"
 #include "chrome/browser/component_updater/component_updater_service.h"
 #include "chrome/browser/component_updater/pnacl/pnacl_profile_observer.h"
+#include "chrome/browser/component_updater/pnacl/pnacl_updater_observer.h"
 
 class CommandLine;
 
@@ -59,12 +61,31 @@ class PnaclComponentInstaller : public ComponentInstaller {
 
   ComponentUpdateService* cus() const { return cus_; }
 
+  typedef base::Callback<void(bool)> InstallCallback;
+  void AddInstallCallback(const InstallCallback& cb);
+
+  void NotifyInstallError();
+
+  void NotifyInstallSuccess();
+
  private:
+  // Cancel a particular callback after a timeout.
+  void CancelCallback(int callback_num);
+
+  void NotifyAllWithResult(bool status);
+
   bool per_user_;
   scoped_ptr<PnaclProfileObserver> profile_observer_;
   base::FilePath current_profile_path_;
   base::Version current_version_;
   ComponentUpdateService* cus_;
+  // Counter to issue identifiers to each callback.
+  int callback_nums_;
+  // List of callbacks to issue when an install completes successfully.
+  std::list<std::pair<InstallCallback, int> > install_callbacks_;
+  // Component updater service observer, to determine when an on-demand
+  // install request failed.
+  scoped_ptr<PnaclUpdaterObserver> updater_observer_;
   DISALLOW_COPY_AND_ASSIGN(PnaclComponentInstaller);
 };
 
@@ -73,4 +94,11 @@ class PnaclComponentInstaller : public ComponentInstaller {
 bool CheckPnaclComponentManifest(const base::DictionaryValue& manifest,
                                  base::Version* version_out);
 
+// Ask the given component updater service to do a first-install for PNaCl.
+// The |installed| callback will be run with |true| on success,
+// or run with |false| on an error. The callback is called on the UI thread.
+void RequestFirstInstall(ComponentUpdateService* cus,
+                         PnaclComponentInstaller* pci,
+                         const base::Callback<void(bool)>& installed);
+
 #endif  // CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_COMPONENT_INSTALLER_H_

chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc

+// Copyright (c) 2013 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_updater_observer.h"
+
+#include "base/logging.h"
+#include "chrome/browser/component_updater/pnacl/pnacl_component_installer.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_service.h"
+
+PnaclUpdaterObserver::PnaclUpdaterObserver(
+    PnaclComponentInstaller* installer) : pnacl_installer_(installer) {
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING,
+                 content::NotificationService::AllSources());
+}
+
+PnaclUpdaterObserver::~PnaclUpdaterObserver() { }
+
+void PnaclUpdaterObserver::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  if (type == chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING) {
+    // If the component updater sleeps before a NotifyInstallSuccess,
+    // then requests for installs were likely skipped, or an error occurred.
+    pnacl_installer_->NotifyInstallError();
+    return;
+  }
+}

chrome/browser/component_updater/pnacl/pnacl_updater_observer.h

+// Copyright (c) 2013 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.
+
+#ifndef CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_
+#define CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_
+
+#include "base/compiler_specific.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class PnaclComponentInstaller;
+
+// Monitors the component updater service, so that the callbacks registered
+// against the PnaclComponentInstaller can be notified of events.
+class PnaclUpdaterObserver : public content::NotificationObserver {
+ public:
+  explicit PnaclUpdaterObserver(PnaclComponentInstaller* installer);
+  virtual ~PnaclUpdaterObserver();
+
+  virtual void Observe(
+      int type,
+      const content::NotificationSource& source,
+      const content::NotificationDetails& details) OVERRIDE;
+
+ private:
+  content::NotificationRegistrar registrar_;
+  PnaclComponentInstaller* pnacl_installer_;
+  DISALLOW_COPY_AND_ASSIGN(PnaclUpdaterObserver);
+};
+
+#endif  // CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_

chrome/browser/nacl_host/nacl_file_host.cc

@@ -11,6 +11,8 @@
 #include "base/platform_file.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/sequenced_worker_pool.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/component_updater/pnacl/pnacl_component_installer.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/nacl_host/nacl_browser.h"
 #include "chrome/browser/nacl_host/nacl_host_message_filter.h"
@@ -55,6 +57,47 @@ bool PnaclDoOpenFile(const base::FilePath& file_to_open,
   return true;
 }
 
+void DoOpenPnaclFile(
+    scoped_refptr<NaClHostMessageFilter> nacl_host_message_filter,
+    const std::string& filename,
+    IPC::Message* reply_msg);
+
+void PnaclCheckDone(
+    scoped_refptr<NaClHostMessageFilter> nacl_host_message_filter,
+    const std::string& filename,
+    IPC::Message* reply_msg,
+    bool success) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!success) {
+    NotifyRendererOfError(nacl_host_message_filter, reply_msg);
+  } else {
+    if (!BrowserThread::PostBlockingPoolTask(
+            FROM_HERE,
+            base::Bind(&DoOpenPnaclFile,
+                       nacl_host_message_filter,
+                       filename,
+                       reply_msg))) {
+      NotifyRendererOfError(nacl_host_message_filter, reply_msg);
+    }
+  }
+}
+
+void TryInstallPnacl(
+    scoped_refptr<NaClHostMessageFilter> nacl_host_message_filter,
+    const std::string& filename,
+    IPC::Message* reply_msg) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  ComponentUpdateService* cus = g_browser_process->component_updater();
+  PnaclComponentInstaller* pci =
+      g_browser_process->pnacl_component_installer();
+  RequestFirstInstall(cus,
+                      pci,
+                      base::Bind(&PnaclCheckDone,
+                                 nacl_host_message_filter,
+                                 filename,
+                                 reply_msg));
+}
+
 void DoOpenPnaclFile(
     scoped_refptr<NaClHostMessageFilter> nacl_host_message_filter,
     const std::string& filename,
@@ -62,6 +105,19 @@ void DoOpenPnaclFile(
   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
   base::FilePath full_filepath;
 
+  // PNaCl must be installed.
+  base::FilePath pnacl_dir;
+  if (!PathService::Get(chrome::DIR_PNACL_COMPONENT, &pnacl_dir) ||
+      !file_util::PathExists(pnacl_dir)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&TryInstallPnacl,
+                   nacl_host_message_filter,
+                   filename,
+                   reply_msg));
+    return;
+  }
+
   // Do some validation.
   if (!nacl_file_host::PnaclCanOpenFile(filename, &full_filepath)) {
     NotifyRendererOfError(nacl_host_message_filter.get(), reply_msg);

chrome/chrome_browser.gypi

@@ -400,6 +400,8 @@
         'browser/component_updater/pnacl/pnacl_component_installer.h',
         'browser/component_updater/pnacl/pnacl_profile_observer.cc',
         'browser/component_updater/pnacl/pnacl_profile_observer.h',
+        'browser/component_updater/pnacl/pnacl_updater_observer.cc',
+        'browser/component_updater/pnacl/pnacl_updater_observer.h',
         'browser/component_updater/recovery_component_installer.cc',
         'browser/component_updater/recovery_component_installer.h',
         'browser/component_updater/swiftshader_component_installer.cc',