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

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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207954

[jvoung (off chromium), 2013-06-14 16:48:12]

This is the on demand CL I rebased from
(https://codereview.chromium.org/12184029/).

There a bunch of opportunities for cleanup if you are interested in working on
this (delay timers are bit long, no UI, and the open+install call is blocking).

Did you want to do look at this more and some clean up first, or should we do
this as is for a first step?

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

Couple of comment issues, but nothing major.  LGTM from my perspective.

https://codereview.chromium.org/17001003/diff/4001/chrome/browser/component_u...
File chrome/browser/component_updater/pnacl/pnacl_component_installer.cc
(right):

https://codereview.chromium.org/17001003/diff/4001/chrome/browser/component_u...
chrome/browser/component_updater/pnacl/pnacl_component_installer.cc:318: // Why
unretained? The installer should have
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?

https://codereview.chromium.org/17001003/diff/4001/chrome/browser/component_u...
chrome/browser/component_updater/pnacl/pnacl_component_installer.cc:351: // Why
unretained? The installer lives until process shutdown.
Same comment issue here.

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

+carlos, could you take a look?

https://codereview.chromium.org/17001003/diff/4001/chrome/browser/component_u...
File chrome/browser/component_updater/pnacl/pnacl_component_installer.cc
(right):

https://codereview.chromium.org/17001003/diff/4001/chrome/browser/component_u...
chrome/browser/component_updater/pnacl/pnacl_component_installer.cc:318: // Why
unretained? The installer should have
On 2013/06/14 18:23:43, sehr1 wrote:
> 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?

Done.

https://codereview.chromium.org/17001003/diff/4001/chrome/browser/component_u...
chrome/browser/component_updater/pnacl/pnacl_component_installer.cc:351: // Why
unretained? The installer lives until process shutdown.
On 2013/06/14 18:23:43, sehr1 wrote:
> Same comment issue here.

Done.

[jvoung (off chromium), 2013-06-18 23:40:20]

Changed a bit to check for updater service's sleep events as a signal for update
failure / skip, instead of using a timeout.

[sehr, 2013-06-20 21:11:42]

On 2013/06/18 23:40:20, jvoung (cr) wrote:
> Changed a bit to check for updater service's sleep events as a signal for
update
> failure / skip, instead of using a timeout.

LGTM from my perspective, again.  Carlos?

[cpu_(ooo_6.6-7.5), 2013-06-20 22:21:44]

lgtm

https://codereview.chromium.org/17001003/diff/29001/chrome/browser/component_...
File chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc (right):

https://codereview.chromium.org/17001003/diff/29001/chrome/browser/component_...
chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc:26: // If the
component updater sleeps before a NotifyInstallSuccess,
oh we better not break this pseudo contract.

Let me know if you think of a way to make this more robust.
\

[jvoung (off chromium), 2013-06-20 22:55:18]

https://codereview.chromium.org/17001003/diff/29001/chrome/browser/component_...
File chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc (right):

https://codereview.chromium.org/17001003/diff/29001/chrome/browser/component_...
chrome/browser/component_updater/pnacl/pnacl_updater_observer.cc:26: // If the
component updater sleeps before a NotifyInstallSuccess,
On 2013/06/20 22:21:45, cpu wrote:
> oh we better not break this pseudo contract.
> 
> Let me know if you think of a way to make this more robust.
> \


One way could be to add a NOTIFICATION_COMPONENT_UPDATE_SUCCESS, w/ the
component ID as the NotificationDetails.

That would just be a way to make the kinds of "notification" uniform and come
from the same source.  Right now one signal is a callback, and the the other
signal is a notification, which makes it hard to be a real contract.

It looks like the notification service is supposed to run synchronously, so it
wouldn't race (I hope).

After receiving a relevant message from the notification service, this observer
would stop observing.  It would only start observing again when
RequestFirstInstall() is called again.

Will think about other ways...

[commit-bot: I haz the power, 2013-06-21 01:17:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/17001003/29001

[commit-bot: I haz the power, 2013-06-21 01:45:33]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-21 04:27:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/17001003/53001

[commit-bot: I haz the power, 2013-06-21 14:57:44]

Retried try job too often on win_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-21 15:00:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/17001003/53001

[commit-bot: I haz the power, 2013-06-21 15:00:47]

Failed to apply patch for
chrome/browser/component_updater/pnacl/pnacl_updater_observer.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
  Copied chrome/browser/component_updater/pnacl/pnacl_profile_observer.h ->
chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
  patching file chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
  Hunk #1 FAILED at 2.
  Hunk #2 succeeded at 26 (offset 1 line).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/component_updater/pnacl/pnacl_updater_observer.h.rej

Patch:   NR 
chrome/browser/component_updater/pnacl/pnacl_profile_observer.h->chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
Index: chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
diff --git a/chrome/browser/component_updater/pnacl/pnacl_profile_observer.h
b/chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
similarity index 53%
copy from chrome/browser/component_updater/pnacl/pnacl_profile_observer.h
copy to chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
index
52ab9847dc1407dc2a66e8e3e6de445d262f8cee..f76668328bd25b21ecc345eb5be5671815fcbfe9
100644
--- a/chrome/browser/component_updater/pnacl/pnacl_profile_observer.h
+++ b/chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
@@ -2,20 +2,20 @@
 // 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_PROFILE_OBSERVER_H_
-#define CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_PROFILE_OBSERVER_H_
+#ifndef CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_
+#define CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_
 
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
 class PnaclComponentInstaller;
 
-// Monitors profile switching for ChromeOS to check the per-user
-// version of PNaCl.
-class PnaclProfileObserver : public content::NotificationObserver {
+// 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 PnaclProfileObserver(PnaclComponentInstaller* installer);
-  virtual ~PnaclProfileObserver();
+  explicit PnaclUpdaterObserver(PnaclComponentInstaller* installer);
+  virtual ~PnaclUpdaterObserver();
 
   virtual void Observe(
       int type,
@@ -25,7 +25,7 @@ class PnaclProfileObserver : public
content::NotificationObserver {
  private:
   content::NotificationRegistrar registrar_;
   PnaclComponentInstaller* pnacl_installer_;
-  DISALLOW_COPY_AND_ASSIGN(PnaclProfileObserver);
+  DISALLOW_COPY_AND_ASSIGN(PnaclUpdaterObserver);
 };
 
-#endif  // CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_PROFILE_OBSERVER_H_
+#endif  // CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_

[commit-bot: I haz the power, 2013-06-21 16:06:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/17001003/86003

[commit-bot: I haz the power, 2013-06-21 20:59:33]

Failed to apply patch for
chrome/browser/component_updater/pnacl/pnacl_updater_observer.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
  Copied chrome/browser/component_updater/pnacl/pnacl_profile_observer.h ->
chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
  patching file chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
  Hunk #1 FAILED at 2.
  Hunk #2 succeeded at 26 (offset 1 line).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/component_updater/pnacl/pnacl_updater_observer.h.rej

Patch:   NR 
chrome/browser/component_updater/pnacl/pnacl_profile_observer.h->chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
Index: chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
diff --git a/chrome/browser/component_updater/pnacl/pnacl_profile_observer.h
b/chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
similarity index 51%
copy from chrome/browser/component_updater/pnacl/pnacl_profile_observer.h
copy to chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
index
52ab9847dc1407dc2a66e8e3e6de445d262f8cee..48a6f0d9c81cbceb527da5478be4d0220ec1638f
100644
--- a/chrome/browser/component_updater/pnacl/pnacl_profile_observer.h
+++ b/chrome/browser/component_updater/pnacl/pnacl_updater_observer.h
@@ -2,20 +2,21 @@
 // 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_PROFILE_OBSERVER_H_
-#define CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_PROFILE_OBSERVER_H_
+#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 profile switching for ChromeOS to check the per-user
-// version of PNaCl.
-class PnaclProfileObserver : public content::NotificationObserver {
+// 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 PnaclProfileObserver(PnaclComponentInstaller* installer);
-  virtual ~PnaclProfileObserver();
+  explicit PnaclUpdaterObserver(PnaclComponentInstaller* installer);
+  virtual ~PnaclUpdaterObserver();
 
   virtual void Observe(
       int type,
@@ -25,7 +26,7 @@ class PnaclProfileObserver : public
content::NotificationObserver {
  private:
   content::NotificationRegistrar registrar_;
   PnaclComponentInstaller* pnacl_installer_;
-  DISALLOW_COPY_AND_ASSIGN(PnaclProfileObserver);
+  DISALLOW_COPY_AND_ASSIGN(PnaclUpdaterObserver);
 };
 
-#endif  // CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_PROFILE_OBSERVER_H_
+#endif  // CHROME_BROWSER_COMPONENT_UPDATER_PNACL_PNACL_UPDATER_OBSERVER_H_

[commit-bot: I haz the power, 2013-06-21 21:53:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jvoung@chromium.org/17001003/81004

[jvoung (off chromium), 2013-06-21 23:31:51]

Committed patchset #9 manually as r207954 (presubmit successful).