GCMDriver.java should start the browser processes if necessary

GCMDriver.java should start the browser processes if necessary

When GCMDriver is woken up for an Intent, now if the browser processes
aren't yet started (along with GCMDriver's corresponding native
counterpart), it will start the browser processes itself.

Depends on https://codereview.chromium.org/314293006/

BUG=350384
R=aberent@chromium.org, fgorski@chromium.org

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

[fgorski, 2014-06-11 22:59:29]

A couple of comments to clarify my understanding of the code.

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
File
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java
(right):

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:202:
private static void launchNativeThen(Runnable task, Context context) {
Is there a convention as to where we pass Context? you have methods taking it as
a first parameter, and at least this one takes it as a last one. Is that ok?

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:217:
Log.e(TAG, "Started browser process, but failed to instantiate GCMDriver.");
From the code I understand that startBrowserProcessesSync will only return after
the browser process was started and all initialization happened, which includes
creation of GCMDriver and initialization of its Android piece (which sets the
sInstance properly)...
Is that what you expect?

Do you expect sInstance != null to happen exclusively on some error?

[aberent, 2014-06-12 17:24:05]

lgtm

[johnme, 2014-06-12 18:09:05]

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
File
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java
(right):

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:202:
private static void launchNativeThen(Runnable task, Context context) {
On 2014/06/11 22:59:29, fgorski wrote:
> Is there a convention as to where we pass Context? you have methods taking it
as
> a first parameter, and at least this one takes it as a last one. Is that ok?

I don't think there's a strict convention, but I've changed launchNativeThen to
take it as a first parameter, for consistency.

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:217:
Log.e(TAG, "Started browser process, but failed to instantiate GCMDriver.");
On 2014/06/11 22:59:28, fgorski wrote:
> From the code I understand that startBrowserProcessesSync will only return
after
> the browser process was started and all initialization happened, which
includes
> creation of GCMDriver and initialization of its Android piece (which sets the
> sInstance properly)...
> Is that what you expect?
> 
> Do you expect sInstance != null to happen exclusively on some error?

I would like sInstance == null to happen exclusively in error cases. Based on
our recent email thread, it's become apparent that since GCMProfileService is
lazily initialized, currently this relies on some other component starting up
GCMDriver during startup (which does currently work, but I shouldn't be relying
on that). I've added a TODO to plumb through JNI a method that makes sure
GCMDriver is launched.

[johnme, 2014-06-12 23:46:55]

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
File
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java
(right):

https://codereview.chromium.org/331443004/diff/1/components/gcm_driver/androi...
components/gcm_driver/android/java/src/org/chromium/components/gcm_driver/GCMDriver.java:217:
Log.e(TAG, "Started browser process, but failed to instantiate GCMDriver.");
On 2014/06/12 18:09:04, johnme wrote:
> On 2014/06/11 22:59:28, fgorski wrote:
> > From the code I understand that startBrowserProcessesSync will only return
> after
> > the browser process was started and all initialization happened, which
> includes
> > creation of GCMDriver and initialization of its Android piece (which sets
the
> > sInstance properly)...
> > Is that what you expect?
> > 
> > Do you expect sInstance != null to happen exclusively on some error?
> 
> I would like sInstance == null to happen exclusively in error cases. Based on
> our recent email thread, it's become apparent that since GCMProfileService is
> lazily initialized, currently this relies on some other component starting up
> GCMDriver during startup (which does currently work, but I shouldn't be
relying
> on that). I've added a TODO to plumb through JNI a method that makes sure
> GCMDriver is launched.

Specifically, when I run startBrowserProcessesSync, GCMProfileService currently
gets started up by ProfileSyncServiceFactory::BuildServiceInstanceFor.

[fgorski, 2014-06-13 00:13:16]

lgtm

[johnme, 2014-06-13 12:53:06]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-06-13 12:53:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/johnme@chromium.org/331443004/40001

[johnme, 2014-06-13 14:52:46]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-06-13 14:54:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/johnme@chromium.org/331443004/60001

[commit-bot: I haz the power, 2014-06-13 18:12:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-13 18:13:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-13 18:13:36]

Try jobs failed on following builders:
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)

[johnme, 2014-06-13 18:41:00]

Committed patchset #4 manually as r277060 (presubmit successful).