Split GCMDriver into platform-specific implementations

This patch splits GCMDriver into an abstract interface and an
implementation thereof for desktop platforms. It also provides a stub
Android implementation, to be fleshed out in subsequent patches.

BUG=350384
TEST=existing tests
TBR=pavely@chromium.org for updates to invalidation tests

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

[johnme, 2014-05-07 18:36:36]

Hi Jian, I've been talking with Filip about how best to provide an Android
backend for GCMProfileService, and he recommended providing an alternate
implementation of GCMDriver. I've put together this patch to demonstrate that.
Does this seem reasonable?

[fgorski, 2014-05-07 18:50:13]

John it looks OK, but I'd like to hear what Jian Li thinks.

From my side: did you try to enable/run the unit tests on Android? We should
probably start thinking early about that, as from now on making changes to
desktop GCM might affect android as well.

https://codereview.chromium.org/278493002/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_profile_service.cc (right):

https://codereview.chromium.org/278493002/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service.cc:82:
DCHECK(!profile->IsOffTheRecord());
There might be a few more headers to ifdef I think. (speculating)

https://codereview.chromium.org/278493002/diff/1/components/gcm.gypi
File components/gcm.gypi (right):

https://codereview.chromium.org/278493002/diff/1/components/gcm.gypi#newcode27
components/gcm.gypi:27: 'gcm/gcm_driver_android.h',
Would this compile the gcm_driver_android on desktop as well? If so, do we need
it?

[jianli, 2014-05-08 00:26:48]

One general question before I could make more comments. For GCM Chrome on
Android, do we need to switch to another thread, like IO thread, in order to
call JNI?

If yes, it seems that we'd better to provide an Android implementation of
GCMClient.

Otherwise, it seems to be better and simpler to strip out the dependency on
GCMClient from GCMDriver. That is, those types defined in GCMClient, like
Result, IncomingMessage and OutgoingMessage should be considered to move to
other place. Maybe we could move these to google_apis/gcm/public.

https://codereview.chromium.org/278493002/diff/1/components/gcm/gcm_driver.h
File components/gcm/gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/1/components/gcm/gcm_driver.h#...
components/gcm/gcm_driver.h:37: GCMDriver(const
scoped_refptr<base::MessageLoopProxy>& ui_thread);
I don't think you need to pass a ui thread to the base class in order to
validate the calling thread. It should be left to the derived class to decide.
For GCMDriverAndroid, it probably doesn't need to perform such checks.

[johnme, 2014-05-08 16:08:02]

Thanks for the reviews!

On 2014/05/07 18:50:13, fgorski wrote:
> John it looks OK, but I'd like to hear what Jian Li thinks.
> 
> From my side: did you try to enable/run the unit tests on Android? We should
> probably start thinking early about that, as from now on making changes to
> desktop GCM might affect android as well.

I haven't yet, but I agree that this is a high priority. Once the stub is in
place, it should be easier to start hooking these up.

On 2014/05/08 00:26:48, jianli wrote:
> One general question before I could make more comments. For GCM Chrome on
> Android, do we need to switch to another thread, like IO thread, in order to
> call JNI?

You can call to JNI on any thread (and Java will execute on that thread; hence
the main browser process thread == the Java UI thread).

The GCM API's register and unregister methods are blocking, so in my
prototype[1], I called to Java on the main thread, then followed the GCM example
code[2] and used a Java AsyncTask[3] to perform the actual registration on an
arbitrary background thread.

Incoming push messages seem to arrive on a non-main thread (though I'm not
certain of this), and so I used ThreadUtils.runOnUIThread[4] to switch back to
the main thread before sending the events back to C++.

This seems to work fine; I guess the C++ equivalent would be to post a task to
BrowserThread::GetBlockingPool() instead of using AsyncTask, but I'm not aware
of any advantage to doing that instead.

[1]: https://codereview.chromium.org/237323002
[2]: http://developer.android.com/google/gcm/client.html#sample-register
[3]:
https://codereview.chromium.org/237323002/diff/1/chrome/android/java/src/org/...
[4]:
https://codereview.chromium.org/237323002/diff/1/chrome/android/java/src/org/...

> If yes, it seems that we'd better to provide an Android implementation of
> GCMClient.

Heh, that's actually what I tried in an earlier iteration of this patch --
https://codereview.chromium.org/246863004/ -- but Filip thought this would
probably be better. It sounds like that matches your assessment too, since
switching to another thread for JNI isn't required.

> Otherwise, it seems to be better and simpler to strip out the dependency on
> GCMClient from GCMDriver. That is, those types defined in GCMClient, like
> Result, IncomingMessage and OutgoingMessage should be considered to move to
> other place. Maybe we could move these to google_apis/gcm/public.

Sounds sensible, though it's perhaps easier to do that in a separate patch. I've
put together https://codereview.chromium.org/270873002 to do this (there are
still dependencies on GCMClient::GCMStatistics and as well as GCMClient itself
thanks to the GetGCMClientForTesting method, but those could presumably be moved
to GCMDriverDesktop later). Does that look good?

https://codereview.chromium.org/278493002/diff/1/chrome/browser/services/gcm/...
File chrome/browser/services/gcm/gcm_profile_service.cc (right):

https://codereview.chromium.org/278493002/diff/1/chrome/browser/services/gcm/...
chrome/browser/services/gcm/gcm_profile_service.cc:82:
DCHECK(!profile->IsOffTheRecord());
On 2014/05/07 18:50:14, fgorski wrote:
> There might be a few more headers to ifdef I think. (speculating)

Done. Also removed a bunch of unused headers from gcm_driver.cc.

https://codereview.chromium.org/278493002/diff/1/components/gcm.gypi
File components/gcm.gypi (right):

https://codereview.chromium.org/278493002/diff/1/components/gcm.gypi#newcode27
components/gcm.gypi:27: 'gcm/gcm_driver_android.h',
On 2014/05/07 18:50:14, fgorski wrote:
> Would this compile the gcm_driver_android on desktop as well? If so, do we
need
> it?

No, *_android.* is only compiled on Android. See [1] (needs updating to mention
_android, but the same mechanism does work on Android). For gcm_driver_desktop,
I wasn't able to use that mechanism, so I used the preferred alternative
mechanism listed below on that page, which is to list it but then exclude it.

[1]:
https://code.google.com/p/gyp/wiki/GypUserDocumentation#Add_a_platform-specif...

https://codereview.chromium.org/278493002/diff/1/components/gcm/gcm_driver.h
File components/gcm/gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/1/components/gcm/gcm_driver.h#...
components/gcm/gcm_driver.h:37: GCMDriver(const
scoped_refptr<base::MessageLoopProxy>& ui_thread);
On 2014/05/08 00:26:48, jianli wrote:
> I don't think you need to pass a ui thread to the base class in order to
> validate the calling thread. It should be left to the derived class to decide.
> For GCMDriverAndroid, it probably doesn't need to perform such checks.

Done.

[johnme, 2014-05-28 15:46:22]

Hi Jian & Filip, I've rebased (well, rewritten!) this to take into account the
various http://crbug.com/356716 componentization refactorings (including
https://codereview.chromium.org/290013011/).

I also added a FakeGCMDriver for use by
chrome/browser/invalidation/gcm_invalidation_bridge_unittest.cc,
chrome/browser/invalidation/ticl_invalidation_service_unittest.cc and
chrome/browser/services/gcm/fake_gcm_profile_service.cc, which seems cleaner
than my previous approach where each of their FakeGCMDrivers inherited from
GCMDriverDesktop.

PTAL :-)

[jianli, 2014-05-29 00:54:53]

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver.h:38: virtual void Enable() = 0;
Move these 2 methods down to group with all other abstract methods.

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver.h:107: base::ThreadChecker thread_checker_;
Why do we need this? We can defer the thread checking logic to the child class
if needed.

[jianli, 2014-05-29 07:34:03]

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/fa...
File components/gcm_driver/fake_gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/fa...
components/gcm_driver/fake_gcm_driver.h:14: // A do-nothing implementation of
GCMDriver.
nit: comment is not needed

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/fa...
components/gcm_driver/fake_gcm_driver.h:45:
DISALLOW_COPY_AND_ASSIGN(FakeGCMDriver);
nit: add private before this line

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_unittest.cc (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_unittest.cc:194: driver_.reset(new
GCMDriverDesktop(
Will this still be built for Android?

[johnme, 2014-05-29 17:40:55]

Addressed Jian's review comments. Thanks :)

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/fa...
File components/gcm_driver/fake_gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/fa...
components/gcm_driver/fake_gcm_driver.h:14: // A do-nothing implementation of
GCMDriver.
On 2014/05/29 07:34:03, jianli wrote:
> nit: comment is not needed

Done (removed).

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/fa...
components/gcm_driver/fake_gcm_driver.h:45:
DISALLOW_COPY_AND_ASSIGN(FakeGCMDriver);
On 2014/05/29 07:34:03, jianli wrote:
> nit: add private before this line

Done (and added a DISALLOW_COPY_AND_ASSIGN to gcm_driver_android.h, which was
missing one).

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver.h:38: virtual void Enable() = 0;
On 2014/05/29 00:54:54, jianli wrote:
> Move these 2 methods down to group with all other abstract methods.

Done (and reordered to match elsewhere).

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver.h:107: base::ThreadChecker thread_checker_;
On 2014/05/29 00:54:54, jianli wrote:
> Why do we need this? We can defer the thread checking logic to the child class
> if needed.

Using ThreadChecker, GCMDriver can easily check that the app handler methods all
get called on the same thread (without having to know what thread that is);
otherwise subclasses would have to override those methods just to add the thread
checks. As it happens, GCMDriverDesktop overrides several of these anyway, but I
was hoping GCMDriverAndroid at least wouldn't need to. Happy to remove this if
you'd rather though.

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_unittest.cc (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_unittest.cc:194: driver_.reset(new
GCMDriverDesktop(
On 2014/05/29 07:34:03, jianli wrote:
> Will this still be built for Android?

Great question! It was, but only by accident because I'd misspelled the
'sources!' exclude for gcm_driver_desktop on Android. I've fixed the exclude, so
gcm_driver_desktop is no longer built on Android, but instead I've added
gcm_driver_desktop to 'sources' for gcm_driver_test_support, so it will get
built on Android for tests only.

This means these tests will continue to check that GCMDriverDesktop works on
Android, even though it's not actually used on Android; the alternative would be
to disable the desktop version of these tests on Android. This boils down to: is
it better to test an unsupported configuration, or lose testing for it?

I'm in two minds; happy to change this to disable the tests on Android instead
of adding gcm_driver_desktop to gcm_driver_test_support if you'd prefer.

[jianli, 2014-05-30 06:30:22]

lgtm

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver.h:107: base::ThreadChecker thread_checker_;
On 2014/05/29 17:40:55, johnme wrote:
> On 2014/05/29 00:54:54, jianli wrote:
> > Why do we need this? We can defer the thread checking logic to the child
class
> > if needed.
> 
> Using ThreadChecker, GCMDriver can easily check that the app handler methods
all
> get called on the same thread (without having to know what thread that is);
> otherwise subclasses would have to override those methods just to add the
thread
> checks. As it happens, GCMDriverDesktop overrides several of these anyway, but
I
> was hoping GCMDriverAndroid at least wouldn't need to. Happy to remove this if
> you'd rather though.

If there is no thread switching needed for GCMDriverAndroid, we probably don't
need to add this concept in GCMDriver.

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_unittest.cc (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_unittest.cc:194: driver_.reset(new
GCMDriverDesktop(
On 2014/05/29 17:40:55, johnme wrote:
> On 2014/05/29 07:34:03, jianli wrote:
> > Will this still be built for Android?
> 
> Great question! It was, but only by accident because I'd misspelled the
> 'sources!' exclude for gcm_driver_desktop on Android. I've fixed the exclude,
so
> gcm_driver_desktop is no longer built on Android, but instead I've added
> gcm_driver_desktop to 'sources' for gcm_driver_test_support, so it will get
> built on Android for tests only.
> 
> This means these tests will continue to check that GCMDriverDesktop works on
> Android, even though it's not actually used on Android; the alternative would
be
> to disable the desktop version of these tests on Android. This boils down to:
is
> it better to test an unsupported configuration, or lose testing for it?
> 
> I'm in two minds; happy to change this to disable the tests on Android instead
> of adding gcm_driver_desktop to gcm_driver_test_support if you'd prefer.

I think we should test GCMDriverAndroid on Android since GCMDriverDesktop is
never used there. If GCMDriverAndroid testing is not ready, we can disable the
tests until they're ready.

https://codereview.chromium.org/278493002/diff/60001/components/gcm_driver.gypi
File components/gcm_driver.gypi (right):

https://codereview.chromium.org/278493002/diff/60001/components/gcm_driver.gy...
components/gcm_driver.gypi:68: 'gcm_driver/gcm_driver_desktop.cc',
I think gcm_driver_desktop.* are included twice for desktop. Will gyp complain
about this?

[johnme, 2014-05-30 10:26:45]

Addressed Jian's review comments - thanks :)

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver.h (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver.h:107: base::ThreadChecker thread_checker_;
On 2014/05/30 06:30:23, jianli wrote:
> On 2014/05/29 17:40:55, johnme wrote:
> > On 2014/05/29 00:54:54, jianli wrote:
> > > Why do we need this? We can defer the thread checking logic to the child
> class
> > > if needed.
> > 
> > Using ThreadChecker, GCMDriver can easily check that the app handler methods
> all
> > get called on the same thread (without having to know what thread that is);
> > otherwise subclasses would have to override those methods just to add the
> thread
> > checks. As it happens, GCMDriverDesktop overrides several of these anyway,
but
> I
> > was hoping GCMDriverAndroid at least wouldn't need to. Happy to remove this
if
> > you'd rather though.
> 
> If there is no thread switching needed for GCMDriverAndroid, we probably don't
> need to add this concept in GCMDriver.

Done (removed).

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
File components/gcm_driver/gcm_driver_unittest.cc (right):

https://codereview.chromium.org/278493002/diff/40001/components/gcm_driver/gc...
components/gcm_driver/gcm_driver_unittest.cc:194: driver_.reset(new
GCMDriverDesktop(
On 2014/05/30 06:30:23, jianli wrote:
> On 2014/05/29 17:40:55, johnme wrote:
> > On 2014/05/29 07:34:03, jianli wrote:
> > > Will this still be built for Android?
> > 
> > Great question! It was, but only by accident because I'd misspelled the
> > 'sources!' exclude for gcm_driver_desktop on Android. I've fixed the
exclude,
> so
> > gcm_driver_desktop is no longer built on Android, but instead I've added
> > gcm_driver_desktop to 'sources' for gcm_driver_test_support, so it will get
> > built on Android for tests only.
> > 
> > This means these tests will continue to check that GCMDriverDesktop works on
> > Android, even though it's not actually used on Android; the alternative
would
> be
> > to disable the desktop version of these tests on Android. This boils down
to:
> is
> > it better to test an unsupported configuration, or lose testing for it?
> > 
> > I'm in two minds; happy to change this to disable the tests on Android
instead
> > of adding gcm_driver_desktop to gcm_driver_test_support if you'd prefer.
> 
> I think we should test GCMDriverAndroid on Android since GCMDriverDesktop is
> never used there. If GCMDriverAndroid testing is not ready, we can disable the
> tests until they're ready.

Done (renamed this file to gcm_driver_desktop_unittest.cc, and disabled it on
Android).

https://codereview.chromium.org/278493002/diff/60001/components/gcm_driver.gypi
File components/gcm_driver.gypi (right):

https://codereview.chromium.org/278493002/diff/60001/components/gcm_driver.gy...
components/gcm_driver.gypi:68: 'gcm_driver/gcm_driver_desktop.cc',
On 2014/05/30 06:30:23, jianli wrote:
> I think gcm_driver_desktop.* are included twice for desktop. Will gyp complain
> about this?

I checked, and it didn't, but I've removed this anyway since we opted not to run
this test on Android until a replacement that tests GCMDriverAndroid is built.

[johnme, 2014-05-30 10:37:45]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-05-30 10:38:42]

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

[commit-bot: I haz the power, 2014-05-30 11:55:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_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/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/10929)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/13997)

[commit-bot: I haz the power, 2014-05-30 11:57:52]

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

[commit-bot: I haz the power, 2014-05-30 11:57:52]

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

[johnme, 2014-05-30 14:52:02]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-05-30 14:53:11]

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

[johnme, 2014-05-30 18:14:25]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-05-30 18:16:10]

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

[commit-bot: I haz the power, 2014-05-31 01:21:14]

Change committed as 273977

[Finnur, 2014-06-02 21:41:45]

Drive-by post-commit comments... :)

https://codereview.chromium.org/278493002/diff/120001/components/gcm_driver/g...
File components/gcm_driver/gcm_driver_desktop.cc (right):

https://codereview.chromium.org/278493002/diff/120001/components/gcm_driver/g...
components/gcm_driver/gcm_driver_desktop.cc:750: GCMClient::IncomingMessage
message) {
Probably not your fault, but could this be passed by ref?

https://codereview.chromium.org/278493002/diff/120001/components/gcm_driver/g...
components/gcm_driver/gcm_driver_desktop.cc:793: GCMClient::GCMStatistics stats)
{
Same here

[jianli, 2014-06-02 23:45:20]

Thanks for noticing this. The fix (
https://codereview.chromium.org/301243019/) is in CQ.


On Mon, Jun 2, 2014 at 2:41 PM, <finnur@chromium.org> wrote:

> Drive-by post-commit comments... :)
>
>
> https://codereview.chromium.org/278493002/diff/120001/
> components/gcm_driver/gcm_driver_desktop.cc
> File components/gcm_driver/gcm_driver_desktop.cc (right):
>
> https://codereview.chromium.org/278493002/diff/120001/
> components/gcm_driver/gcm_driver_desktop.cc#newcode750
> components/gcm_driver/gcm_driver_desktop.cc:750:
> GCMClient::IncomingMessage message) {
> Probably not your fault, but could this be passed by ref?
>
> https://codereview.chromium.org/278493002/diff/120001/
> components/gcm_driver/gcm_driver_desktop.cc#newcode793
> components/gcm_driver/gcm_driver_desktop.cc:793:
> GCMClient::GCMStatistics stats) {
> Same here
>
> https://codereview.chromium.org/278493002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.