Provide GCMService to TiclInvalidationService as an explicit dependency

Provide GCMService to TiclInvalidationService as an explicit dependency

This CL further reduces TiclInvalidationService's dependency on Profile.
Instead of TiclInvalidationService accessing the GCMProfileServiceFactory,
it is now given an GCMService as an explicit dependency.

BUG=362083
TEST=Updated unit tests

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

[bartfab (slow), 2014-04-16 16:37:01]

Hi Daniel,

Could you take a look at chrome/browser/invalidation/*?

Hi Nicolas,

Could you take a look at chrome/browser/services/gcm/*?

[dcheng, 2014-04-16 16:42:18]

LGTM.

What's the advantage to doing this lazily? Can this block startup or something?

[bartfab (slow), 2014-04-16 16:47:15]

On 2014/04/16 16:42:18, dcheng wrote:
> LGTM.
> 
> What's the advantage to doing this lazily? Can this block startup or
something?

I noticed that GCM is not the default transport channel for TICL (as of now). So
at the very least, we would be setting up GCM when it is possibly not going to
be used. Furthermore, the factory actually kicks off Initialize() when it
constructs the GCMProfileService, so it is not just that a GCMProfileService is
constructed but the whole thing is actually started up, occupying more memory
and resources.

It felt wrong to unconditionally do it, especially given that the existing code
lazily requests a GCMProfileService only if needed.

If you feel that GCMProfileService is lightweight enough and/or will be
constructed by someone else anyway, I can make the construction unconditional.
It would simplify things but would have the performance implications outlined
above.

[dcheng, 2014-04-16 16:50:51]

OK, I think you probably have better perspective on this than I do, so it's
fine.

[bartfab (slow), 2014-04-16 17:05:05]

On 2014/04/16 16:50:51, dcheng wrote:
> OK, I think you probably have better perspective on this than I do, so it's
> fine.

Nicolas will be looking at this CL from the GCM perspective. Maybe he can
provide an insight whether it is advisable to unconditionally construct a
GCMProfileService or not.

[Nicolas Zea, 2014-04-16 17:49:52]

GCM is already being kicked off at startup if you're signed in (via sync), and
in m36 is planned to be the only channel by which invalidations are received, so
I think it's fine to unconditionally call GetForProfile

[bartfab (slow), 2014-04-16 18:35:59]

As advised, I made GCMService a construction-time dependency. This simplifies
the code.

[dcheng, 2014-04-16 18:39:54]

On 2014/04/16 18:35:59, bartfab wrote:
> As advised, I made GCMService a construction-time dependency. This simplifies
> the code.

Still LGTM

[Nicolas Zea, 2014-04-16 19:01:08]

lgtm

[bartfab (slow), 2014-04-28 14:21:44]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-04-28 14:22:09]

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

[commit-bot: I haz the power, 2014-04-28 15:01:48]

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

[commit-bot: I haz the power, 2014-04-28 15:01:48]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[bartfab (slow), 2014-04-28 15:08:06]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-04-28 15:08:51]

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

[commit-bot: I haz the power, 2014-04-28 15:36:26]

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

[commit-bot: I haz the power, 2014-04-28 15:36:27]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[bartfab (slow), 2014-04-28 16:51:17]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-04-28 16:51:24]

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

[commit-bot: I haz the power, 2014-04-28 17:36:31]

Change committed as 266596