Merging chrome/common/gcm_desktop_util.* and chrome/browser/services/gcm/gcm_desktop_util.*.

Merging chrome/common/gcm_desktop_util.* and chrome/browser/services/gcm/gcm_desktop_util.*.

The code inside chrome/common/gcm_desktop_util.* only used by the
browser. So there is no point to keep this code inside chrome/common/ folder.

So merging this code to chrome/browser/services/gcm/gcm_desktop_util.*.

BUG=519571
Committed: https://crrev.com/fd2ed4305c304e907238956aa235e7cc5b39d1bc
Cr-Commit-Position: refs/heads/master@{#344966}

[Jitu( very slow this week), 2015-08-19 09:16:25]

jitendra.ks@samsung.com changed reviewers:
+ droger@chromium.org, vivekg@chromium.org

[Jitu( very slow this week), 2015-08-19 09:16:26]

PTAL...

[droger, 2015-08-19 09:52:38]

droger@chromium.org changed reviewers:
+ zea@chromium.org

[droger, 2015-08-19 09:52:38]

Very good. Only a minor comment below.

+zea as OWNER. zea: the goal of this CL is to unblock the componentization of
chrome/common/sync_util, which then eventually unblocks the componentization of
gcm_desktop_util

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

https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
chrome/browser/services/gcm/gcm_desktop_utils.cc:87: scoped_ptr<GCMDriver>
CreateGCMDriverDesktopWithTaskRunners(
Maybe this could be moved to the anonymous namespace too?

[Jitu( very slow this week), 2015-08-19 10:06:03]

On 2015/08/19 09:52:38, droger wrote:
> Very good. Only a minor comment below.
> 
> +zea as OWNER. zea: the goal of this CL is to unblock the componentization of
> chrome/common/sync_util, which then eventually unblocks the componentization
of
> gcm_desktop_util
> 
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> File chrome/browser/services/gcm/gcm_desktop_utils.cc (right):
> 
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> chrome/browser/services/gcm/gcm_desktop_utils.cc:87: scoped_ptr<GCMDriver>
> CreateGCMDriverDesktopWithTaskRunners(
> Maybe this could be moved to the anonymous namespace too?

[Jitu( very slow this week), 2015-08-19 10:06:37]

On 2015/08/19 10:06:03, jitu wrote:
> On 2015/08/19 09:52:38, droger wrote:
> > Very good. Only a minor comment below.
> > 
> > +zea as OWNER. zea: the goal of this CL is to unblock the componentization
of
> > chrome/common/sync_util, which then eventually unblocks the componentization
> of
> > gcm_desktop_util
> > 
> >
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> > File chrome/browser/services/gcm/gcm_desktop_utils.cc (right):
> > 
> >
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> > chrome/browser/services/gcm/gcm_desktop_utils.cc:87: scoped_ptr<GCMDriver>
> > CreateGCMDriverDesktopWithTaskRunners(
> > Maybe this could be moved to the anonymous namespace too?

[Jitu( very slow this week), 2015-08-19 10:06:58]

On 2015/08/19 10:06:37, jitu wrote:
> On 2015/08/19 10:06:03, jitu wrote:
> > On 2015/08/19 09:52:38, droger wrote:
> > > Very good. Only a minor comment below.
> > > 
> > > +zea as OWNER. zea: the goal of this CL is to unblock the componentization
> of
> > > chrome/common/sync_util, which then eventually unblocks the
componentization
> > of
> > > gcm_desktop_util
> > > 
> > >
> >
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> > > File chrome/browser/services/gcm/gcm_desktop_utils.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> > > chrome/browser/services/gcm/gcm_desktop_utils.cc:87: scoped_ptr<GCMDriver>
> > > CreateGCMDriverDesktopWithTaskRunners(
> > > Maybe this could be moved to the anonymous namespace too?

Actually we have declared this in header as part of namespace gcm.
So either we have to keep it like this.

Or if we want to move this inside anonymous namespace then we have to remove the
declaration from header.

Please suggest.

[Jitu( very slow this week), 2015-08-19 10:24:17]

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

https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
chrome/browser/services/gcm/gcm_desktop_utils.cc:87: scoped_ptr<GCMDriver>
CreateGCMDriverDesktopWithTaskRunners(
On 2015/08/19 09:52:38, droger wrote:
> Maybe this could be moved to the anonymous namespace too?

Actually we have declared this in header as part of namespace gcm.
So either we have to keep it like this.

Or if we want to move this inside anonymous namespace then we have to remove the
declaration from header.

Please suggest.

[droger, 2015-08-19 10:40:52]

On 2015/08/19 10:24:17, jitu wrote:
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> File chrome/browser/services/gcm/gcm_desktop_utils.cc (right):
> 
>
https://codereview.chromium.org/1297503003/diff/1/chrome/browser/services/gcm...
> chrome/browser/services/gcm/gcm_desktop_utils.cc:87: scoped_ptr<GCMDriver>
> CreateGCMDriverDesktopWithTaskRunners(
> On 2015/08/19 09:52:38, droger wrote:
> > Maybe this could be moved to the anonymous namespace too?
> 
> Actually we have declared this in header as part of namespace gcm.
> So either we have to keep it like this.
> 
> Or if we want to move this inside anonymous namespace then we have to remove
the
> declaration from header.
> 
> Please suggest.

I was thinking that we could remove it from the header if it's not used.

[Jitu( very slow this week), 2015-08-19 11:01:49]

Fixed the comments

PTAL ...

[droger, 2015-08-19 11:05:00]

lgtm

[Nicolas Zea, 2015-08-19 16:55:16]

The CQ bit was checked by zea@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-19 16:55:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1297503003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1297503003/20001

[Nicolas Zea, 2015-08-19 16:56:15]

https://codereview.chromium.org/1297503003/diff/20001/chrome/browser/services...
File chrome/browser/services/gcm/gcm_desktop_utils.cc (right):

https://codereview.chromium.org/1297503003/diff/20001/chrome/browser/services...
chrome/browser/services/gcm/gcm_desktop_utils.cc:85: scoped_ptr<GCMDriver>
CreateGCMDriverDesktopWithTaskRunners(
nit: Considering this is only used in one place and in an anon namespace, I
think it would be cleaner to just inline it directly in CreateGCMDriverDesktop

[commit-bot: I haz the power, 2015-08-19 17:53:28]

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

[commit-bot: I haz the power, 2015-08-19 17:53:29]

Dry run: This issue passed the CQ dry run.

[Jitu( very slow this week), 2015-08-20 13:45:31]

Patchset #3 (id:40001) has been deleted

[Jitu( very slow this week), 2015-08-20 13:47:41]

PTAL ...

https://codereview.chromium.org/1297503003/diff/20001/chrome/browser/services...
File chrome/browser/services/gcm/gcm_desktop_utils.cc (right):

https://codereview.chromium.org/1297503003/diff/20001/chrome/browser/services...
chrome/browser/services/gcm/gcm_desktop_utils.cc:85: scoped_ptr<GCMDriver>
CreateGCMDriverDesktopWithTaskRunners(
On 2015/08/19 16:56:14, Nicolas Zea wrote:
> nit: Considering this is only used in one place and in an anon namespace, I
> think it would be cleaner to just inline it directly in CreateGCMDriverDesktop

Done.

[Nicolas Zea, 2015-08-20 16:33:44]

LGTM, thanks!

[Jitu( very slow this week), 2015-08-20 16:42:49]

The CQ bit was checked by jitendra.ks@samsung.com

[Jitu( very slow this week), 2015-08-20 16:42:50]

The patchset sent to the CQ was uploaded after l-g-t-m from droger@chromium.org
Link to the patchset: https://codereview.chromium.org/1297503003/#ps60001
(title: "fixed comments")

[commit-bot: I haz the power, 2015-08-20 16:44:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1297503003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1297503003/60001

[commit-bot: I haz the power, 2015-08-20 16:53:08]

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

[commit-bot: I haz the power, 2015-08-20 16:53:09]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Jitu( very slow this week), 2015-08-21 07:06:56]

jitendra.ks@samsung.com changed reviewers:
+ thestig@chromium.org

[Jitu( very slow this week), 2015-08-21 07:06:57]

Dear Lei Zhang,

Could you please do the owner review for 
-- chrome/common/*

[Michael van Ouwerkerk, 2015-08-21 13:58:44]

Note that I was going to use the common bit from the service process, that's why
I moved it there. That project has been stopped though, so go ahead :-)

[sdefresne, 2015-08-21 14:49:11]

On 2015/08/21 at 13:58:44, mvanouwerkerk wrote:
> Note that I was going to use the common bit from the service process, that's
why I moved it there. That project has been stopped though, so go ahead :-)

Moving the code into //chrome/browser/services/gcm means that it won't be
shareable with iOS.
Why not componentize the bits that can be instead?

[sdefresne, 2015-08-21 14:58:04]

On 2015/08/21 at 14:49:11, sdefresne wrote:
> On 2015/08/21 at 13:58:44, mvanouwerkerk wrote:
> > Note that I was going to use the common bit from the service process, that's
why I moved it there. That project has been stopped though, so go ahead :-)
> 
> Moving the code into //chrome/browser/services/gcm means that it won't be
shareable with iOS.
> Why not componentize the bits that can be instead?

Discussed offline. Looks good. Thank you for merging those two files.

[Lei Zhang, 2015-08-21 19:34:48]

lgtm

[Jitu( very slow this week), 2015-08-22 01:59:26]

The CQ bit was checked by jitendra.ks@samsung.com

[commit-bot: I haz the power, 2015-08-22 01:59:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1297503003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1297503003/60001

[commit-bot: I haz the power, 2015-08-22 03:29:11]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-08-22 03:29:43]

Patchset 3 (id:??) landed as
https://crrev.com/fd2ed4305c304e907238956aa235e7cc5b39d1bc
Cr-Commit-Position: refs/heads/master@{#344966}