Introduce a layering in the invalidation component as public and impl

Introduce a layering in the invalidation component as public and impl.

The invalidation component has dependencies due to specific
implementations of the inv_client, inv_service etc.
Consumers of invalidations do not need to depend on these.

This change exports the InvalidationHandler interface in a separate target
'invalidation_public' which only depends on //google/cacheinvalidation

Thus the signin component can implement to InvalidationHandler to refresh
account information without introducing a cyclic dependency.

The crux of the change is:

Move {components/invalidation/ => components/invalidation/public}:
-ack_handle.cc
-ack_handle.h
-ack_handler.cc
-ack_handler.h
-invalidation.cc
-invalidation.h
-invalidation_export.h
-invalidation_handler.cc
-invalidation_handler.h
-invalidation_service.h
-invalidation_util.cc
-invalidation_util.h
-invalidator_state.cc
-invalidator_state.h
-object_id_invalidation_map.cc
-object_id_invalidation_map.h
-single_object_invalidation_set.cc
-single_object_invalidation_set.h

Move {components/invalidation/ => components/invalidation/impl}:
other files

Thus the dependency chain is:
//chrome/browser/ -> //components/invalidation/impl -> components/invalidation/public

Now invalidation consumers can directly depend on //components/invalidation/public
which has only //google/cacheinvalidation has a dependency.

BUG=503131
Committed: https://crrev.com/062cdbbade5d2c804bce7ff7b320d36cb97c39da
Cr-Commit-Position: refs/heads/master@{#336413}

[knn, 2015-06-23 10:23:43]

knn@chromium.org changed reviewers:
+ bauerb@chromium.org

[knn, 2015-06-23 10:23:43]

WDYT? I am not moving files around for now.

[Bernhard Bauer, 2015-06-23 11:36:09]

The CL description doesn't seem to match the contents, and/or I don't quite
understand it. Where is the invalidation_handler target? What do you mean by
implementing the target in a different component? A target is a build step, not
an interface.

It's also a bit weird to have these two targets with a dependency in one
direction, but nothing enforcing that dependency. Usually we move files into
separate directories for that, with DEPS rules to enforce the dependencies.

[knn, 2015-06-23 15:37:43]

Patchset #2 (id:20001) has been deleted

[knn, 2015-06-23 16:10:11]

On 2015/06/23 11:36:09, Bernhard Bauer wrote:
> The CL description doesn't seem to match the contents, and/or I don't quite
> understand it. Where is the invalidation_handler target? What do you mean by
> implementing the target in a different component? A target is a build step,
not
> an interface.
> 
> It's also a bit weird to have these two targets with a dependency in one
> direction, but nothing enforcing that dependency. Usually we move files into
> separate directories for that, with DEPS rules to enforce the dependencies.

Yeah I mean't the interface and not target. Perhaps using pascal_case was
misleading?
I have rephrased the description, Hope it is more clear now.

I wanted to defer moving files around to a follow up CL but DEPS rules is a
valid point.
If it's alright I'd like to defer moving the rest of the files (maybe to
browser/) to a follow up CL.

PTAL.

[knn, 2015-06-23 19:44:59]

knn@chromium.org changed reviewers:
+ pavely@chromium.org

[knn, 2015-06-23 19:45:00]

@Pavel:
Crux of the changes are in 
components/invalidation/BUILD.gn and components/invalidation.gypi

[Bernhard Bauer, 2015-06-24 16:38:24]

So, if you want to move the rest of the files from components/invalidation/ to
c/i/browser/ anyway, what's the point of this CL? After that,
components/invalidation/ will contain only base/, right? Wouldn't we then have
to move everything in base/ back into the main components/invalidation/
directory?

https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
File components/invalidation/base/DEPS (right):

https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
components/invalidation/base/DEPS:4: "+google/cacheinvalidation",
I don't think either of these rules is necessary, as they are already in the
parent DEPS file, and the rules are additive. What you do want is probably a
rule that disallows the parent directory (and then another rule that re-allows
the current directory).

You might also want to disallow components/signin, to prevent cyclic
dependencies.

[knn, 2015-06-24 17:46:25]

knn@chromium.org changed reviewers:
+ dcheng@chromium.org

[knn, 2015-06-24 17:46:27]

@Invalidation Owners:

I want to use invalidation to refresh information to be used in the signin
component.
Since signin is a dependency for the invalidation component, I thought of
introducing a layering with minimal dependency that could be used by
implementors of InvalidationHandler (aka subscribers of invalidation)
The layers are:
components/invalidation/base (only depends on //google/cacheinvalidation)
components/invalidation/browser
The names are only suggestive and I am happy to rename them, but after that
signin can now depend on the invalidation/base layer.

Do you think this sounds reasonable?

I generally like to show you the code if it's reasonably simple, but in this
case properly enforcing DEPS would mean that I have to rename tons of files and
their includes which will make the changes more difficult to parse.

Thanks!

[dcheng, 2015-06-24 20:37:40]

Is the cyclic dependency just //components/signin -> //components/invalidation
-> //components/signin?

[knn, 2015-06-24 20:48:54]

On 2015/06/24 20:37:40, dcheng wrote:
> Is the cyclic dependency just //components/signin -> //components/invalidation
> -> //components/signin?

Presently it is //components/invalidation -> //components/signin/core/browser

I want to:
split to //components/invalidation/base -> //google/cacheinvalidation and
//components/invalidation/browser -> //components/invalidation/base + (other
components, incl //components/signin/core/browser)
add //components/signin/core/browser -> //components/invalidation/base

[dcheng, 2015-06-24 20:52:16]

On 2015/06/24 at 20:48:54, knn wrote:
> On 2015/06/24 20:37:40, dcheng wrote:
> > Is the cyclic dependency just //components/signin ->
//components/invalidation
> > -> //components/signin?
> 
> Presently it is //components/invalidation -> //components/signin/core/browser
> 
> I want to:
> split to //components/invalidation/base -> //google/cacheinvalidation and
> //components/invalidation/browser -> //components/invalidation/base + (other
components, incl //components/signin/core/browser)
> add //components/signin/core/browser -> //components/invalidation/base

Well, I'm asking, because it looks like only one thing depends on signin
(gcm_invalidation_bridge) so I'm wondering if we can just move that to a
separate component rather than moving everything around?

[knn, 2015-06-24 21:06:43]

On 2015/06/24 20:52:16, dcheng wrote:
> On 2015/06/24 at 20:48:54, knn wrote:
> > On 2015/06/24 20:37:40, dcheng wrote:
> > > Is the cyclic dependency just //components/signin ->
> //components/invalidation
> > > -> //components/signin?
> > 
> > Presently it is //components/invalidation ->
//components/signin/core/browser
> > 
> > I want to:
> > split to //components/invalidation/base -> //google/cacheinvalidation and
> > //components/invalidation/browser -> //components/invalidation/base + (other
> components, incl //components/signin/core/browser)
> > add //components/signin/core/browser -> //components/invalidation/base
> 
> Well, I'm asking, because it looks like only one thing depends on signin
> (gcm_invalidation_bridge) so I'm wondering if we can just move that to a
> separate component rather than moving everything around?

gcm_invalidation_bridge has it's own dependencies like ticl_invalidation_service
which again trickles down further so even then we would be moving a bunch of
things around.

My rationale for choosing this split was any other component interested in
invalidation would probably be interested in InvalidationHandler and friends
over anything else and it is good to expose a minimal dependency layer for this
use case. Let me know if I missed some reasoning to prefer the other spilt.

[dcheng, 2015-06-24 21:56:16]

On 2015/06/24 at 21:06:43, knn wrote:
> On 2015/06/24 20:52:16, dcheng wrote:
> > On 2015/06/24 at 20:48:54, knn wrote:
> > > On 2015/06/24 20:37:40, dcheng wrote:
> > > > Is the cyclic dependency just //components/signin ->
> > //components/invalidation
> > > > -> //components/signin?
> > > 
> > > Presently it is //components/invalidation ->
//components/signin/core/browser
> > > 
> > > I want to:
> > > split to //components/invalidation/base -> //google/cacheinvalidation and
> > > //components/invalidation/browser -> //components/invalidation/base +
(other
> > components, incl //components/signin/core/browser)
> > > add //components/signin/core/browser -> //components/invalidation/base
> > 
> > Well, I'm asking, because it looks like only one thing depends on signin
> > (gcm_invalidation_bridge) so I'm wondering if we can just move that to a
> > separate component rather than moving everything around?
> 
> gcm_invalidation_bridge has it's own dependencies like
ticl_invalidation_service which again trickles down further so even then we
would be moving a bunch of things around.
> 
> My rationale for choosing this split was any other component interested in
invalidation would probably be interested in InvalidationHandler and friends
over anything else and it is good to expose a minimal dependency layer for this
use case. Let me know if I missed some reasoning to prefer the other spilt.

OK, fair enough. I just figured doing it the other way would be less work (it's
just gcm_invalidation_bridge.cc and ticl_invalidation_servece.cc) but this is
cleaner. However, I'm not a fan of the proposed naming. The stuff in base/ is
just as browser-specific as the stuff you're proposing on putting in browser/. I
don't have better suggestions for the name though... maybe s/base/public?

pavely@, any thoughts?

[pavely, 2015-06-24 23:42:05]

On 2015/06/24 21:56:16, dcheng wrote:
> On 2015/06/24 at 21:06:43, knn wrote:
> > On 2015/06/24 20:52:16, dcheng wrote:
> > > On 2015/06/24 at 20:48:54, knn wrote:
> > > > On 2015/06/24 20:37:40, dcheng wrote:
> > > > > Is the cyclic dependency just //components/signin ->
> > > //components/invalidation
> > > > > -> //components/signin?
> > > > 
> > > > Presently it is //components/invalidation ->
> //components/signin/core/browser
> > > > 
> > > > I want to:
> > > > split to //components/invalidation/base -> //google/cacheinvalidation
and
> > > > //components/invalidation/browser -> //components/invalidation/base +
> (other
> > > components, incl //components/signin/core/browser)
> > > > add //components/signin/core/browser -> //components/invalidation/base
> > > 
> > > Well, I'm asking, because it looks like only one thing depends on signin
> > > (gcm_invalidation_bridge) so I'm wondering if we can just move that to a
> > > separate component rather than moving everything around?
> > 
> > gcm_invalidation_bridge has it's own dependencies like
> ticl_invalidation_service which again trickles down further so even then we
> would be moving a bunch of things around.
> > 
> > My rationale for choosing this split was any other component interested in
> invalidation would probably be interested in InvalidationHandler and friends
> over anything else and it is good to expose a minimal dependency layer for
this
> use case. Let me know if I missed some reasoning to prefer the other spilt.
> 
> OK, fair enough. I just figured doing it the other way would be less work
(it's
> just gcm_invalidation_bridge.cc and ticl_invalidation_servece.cc) but this is
> cleaner. However, I'm not a fan of the proposed naming. The stuff in base/ is
> just as browser-specific as the stuff you're proposing on putting in browser/.
I
> don't have better suggestions for the name though... maybe s/base/public?
> 
> pavely@, any thoughts?

I think this change is a good split of invalidation code.
Here are my thoughts about naming. Files in base are for consumers of
invalidations. Files proposed to be in browser contain specific implementation
that uses cacheinvalidations library and GCM/XMPP channel.
I agree with Daniel, I would rename "base" to "public" or "api" (I think
"public" is better), sync already follows this model. I would rename "browser"
to "impl".

[pavely, 2015-06-24 23:45:17]

https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
File components/invalidation.gypi (right):

https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
components/invalidation.gypi:31: 'invalidation/base/invalidation_handler.cc',
Could you add invalidation/base/invalidation_export.h here? For some reason it
was not mentioned in this file.

https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
File components/invalidation/BUILD.gn (right):

https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
components/invalidation/BUILD.gn:17: "base/invalidation_handler.h",
Could you add base/invalidation_export.h here too?

[knn, 2015-06-25 13:24:13]

On 2015/06/24 23:45:17, pavely wrote:
>
https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
> File components/invalidation.gypi (right):
> 
>
https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
> components/invalidation.gypi:31: 'invalidation/base/invalidation_handler.cc',
> Could you add invalidation/base/invalidation_export.h here? For some reason it
> was not mentioned in this file.
> 
>
https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
> File components/invalidation/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1191393008/diff/80001/components/invalidation...
> components/invalidation/BUILD.gn:17: "base/invalidation_handler.h",
> Could you add base/invalidation_export.h here too?

Done.

Thanks for the review!
I have split the files into layers c/i/public + c/i/impl and updated DEPS to
enforce that.
PTAL.

[knn, 2015-06-25 13:36:14]

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

[commit-bot: I haz the power, 2015-06-25 13:37:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1191393008/140001

[commit-bot: I haz the power, 2015-06-25 13:50:04]

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

[commit-bot: I haz the power, 2015-06-25 13:50:06]

Dry run: 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...)

[dcheng, 2015-06-25 18:00:37]

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File components/invalidation/DEPS (left):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/DEPS:2: "-content",
I think this should still be present.

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java
(right):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java:22:
import org.chromium.base.Log;
Why make these changes in this patch?

[knn, 2015-06-25 18:20:21]

Thanks Daniel for going through a very long change!
PTAL.

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File components/invalidation/DEPS (left):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/DEPS:2: "-content",
On 2015/06/25 18:00:37, dcheng wrote:
> I think this should still be present.

This is a stale value as content is no longer present by default at the parent
folders.

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java
(right):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java:22:
import org.chromium.base.Log;
On 2015/06/25 18:00:37, dcheng wrote:
> Why make these changes in this patch?

It is a minor change that saves a presubmit warning.

I did this
//components/invalidation/android/java/src/org/chromium/components/invalidation/PendingInvalidation.java
as well.

I added this to the commit message now, does that address your concern or do you
want me to revert this?

[dcheng, 2015-06-25 18:28:31]

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File components/invalidation/DEPS (left):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/DEPS:2: "-content",
On 2015/06/25 at 18:20:21, knn wrote:
> On 2015/06/25 18:00:37, dcheng wrote:
> > I think this should still be present.
> 
> This is a stale value as content is no longer present by default at the parent
folders.

I don't understand what you mean.

The previous policy was the components can't depend on content, and this is what
this DEPS line is supposed to enforce.

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java
(right):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java:22:
import org.chromium.base.Log;
On 2015/06/25 at 18:20:21, knn wrote:
> On 2015/06/25 18:00:37, dcheng wrote:
> > Why make these changes in this patch?
> 
> It is a minor change that saves a presubmit warning.
> 
> I did this
//components/invalidation/android/java/src/org/chromium/components/invalidation/PendingInvalidation.java
as well.
> 
> I added this to the commit message now, does that address your concern or do
you want me to revert this?

It would be nice to put this in a separate change. It's not too hard with git
(you can squash your commits and then use git add -p to cherry-pick just these
diffs out into another patch).

[knn, 2015-06-25 18:48:51]

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File components/invalidation/DEPS (left):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/DEPS:2: "-content",
On 2015/06/25 18:28:31, dcheng wrote:
> On 2015/06/25 at 18:20:21, knn wrote:
> > On 2015/06/25 18:00:37, dcheng wrote:
> > > I think this should still be present.
> > 
> > This is a stale value as content is no longer present by default at the
parent
> folders.
> 
> I don't understand what you mean.
> 
> The previous policy was the components can't depend on content, and this is
what
> this DEPS line is supposed to enforce.

You can only remove a dep that has been added before(inherited from parent
folder).
content has been removed from the parent folder and this removal is a stale
value and must be removed.

Same with net/base/mock_host_resolver which is no longer being used here.

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java
(right):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/impl/android/java/src/org/chromium/components/invalidation/InvalidationClientService.java:22:
import org.chromium.base.Log;
On 2015/06/25 18:28:31, dcheng wrote:
> On 2015/06/25 at 18:20:21, knn wrote:
> > On 2015/06/25 18:00:37, dcheng wrote:
> > > Why make these changes in this patch?
> > 
> > It is a minor change that saves a presubmit warning.
> > 
> > I did this
>
//components/invalidation/android/java/src/org/chromium/components/invalidation/PendingInvalidation.java
> as well.
> > 
> > I added this to the commit message now, does that address your concern or do
> you want me to revert this?
> 
> It would be nice to put this in a separate change. It's not too hard with git
> (you can squash your commits and then use git add -p to cherry-pick just these
> diffs out into another patch).

Will do.

[knn, 2015-06-25 19:34:32]

knn@chromium.org changed reviewers:
+ erg@chromium.org, rogerta@chromium.org, rsleevi@chromium.org,
sdefresne@chromium.org, sky@chromium.org, zea@chromium.org

[knn, 2015-06-25 19:34:34]

Adding more owners:

DEPS file moved:
zea@ for //components/gcm_driver/, //jingle/notifier, //sync, //google_apis/gcm
rogerta@ //components/signin, //google_apis/gaia
erg@ for //components/keyed_service
rsleevi@ for //net/{base/backoff_entry, base/network_change_notifier,
http/http_status_code, url_request}
bauerb@ for //components/pref_registry

Affected import sites:
sky@ for //chrome/
sdefresne@ for //components/test/

[Elliot Glaysher, 2015-06-25 19:53:23]

trivial deps file move lgtm

[dcheng, 2015-06-25 20:37:25]

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
File components/invalidation/DEPS (left):

https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
components/invalidation/DEPS:2: "-content",
On 2015/06/25 at 18:48:51, knn wrote:
> On 2015/06/25 18:28:31, dcheng wrote:
> > On 2015/06/25 at 18:20:21, knn wrote:
> > > On 2015/06/25 18:00:37, dcheng wrote:
> > > > I think this should still be present.
> > > 
> > > This is a stale value as content is no longer present by default at the
parent
> > folders.
> > 
> > I don't understand what you mean.
> > 
> > The previous policy was the components can't depend on content, and this is
what
> > this DEPS line is supposed to enforce.
> 
> You can only remove a dep that has been added before(inherited from parent
folder).
> content has been removed from the parent folder and this removal is a stale
value and must be removed.
> 
> Same with net/base/mock_host_resolver which is no longer being used here.

So this has been broken for awhile? Sorry for the questions, just trying to
understand this.

[knn, 2015-06-25 20:53:45]

On 2015/06/25 20:37:25, dcheng wrote:
>
https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
> File components/invalidation/DEPS (left):
> 
>
https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
> components/invalidation/DEPS:2: "-content",
> On 2015/06/25 at 18:48:51, knn wrote:
> > On 2015/06/25 18:28:31, dcheng wrote:
> > > On 2015/06/25 at 18:20:21, knn wrote:
> > > > On 2015/06/25 18:00:37, dcheng wrote:
> > > > > I think this should still be present.
> > > > 
> > > > This is a stale value as content is no longer present by default at the
> parent
> > > folders.
> > > 
> > > I don't understand what you mean.
> > > 
> > > The previous policy was the components can't depend on content, and this
is
> what
> > > this DEPS line is supposed to enforce.
> > 
> > You can only remove a dep that has been added before(inherited from parent
> folder).
> > content has been removed from the parent folder and this removal is a stale
> value and must be removed.
> > 
> > Same with net/base/mock_host_resolver which is no longer being used here.
> 
> So this has been broken for awhile? Sorry for the questions, just trying to
> understand this.

Not at all, happy to answer. Yes this is an unrequired value since awhile like
the other one from net which wasn't updated.
You can see a similar change here https://codereview.chromium.org/25727006

Basically without //content being added at a top level directory, there is no
reason to remove it here.

[dcheng, 2015-06-25 21:04:46]

On 2015/06/25 at 20:53:45, knn wrote:
> On 2015/06/25 20:37:25, dcheng wrote:
> >
https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
> > File components/invalidation/DEPS (left):
> > 
> >
https://codereview.chromium.org/1191393008/diff/140001/components/invalidatio...
> > components/invalidation/DEPS:2: "-content",
> > On 2015/06/25 at 18:48:51, knn wrote:
> > > On 2015/06/25 18:28:31, dcheng wrote:
> > > > On 2015/06/25 at 18:20:21, knn wrote:
> > > > > On 2015/06/25 18:00:37, dcheng wrote:
> > > > > > I think this should still be present.
> > > > > 
> > > > > This is a stale value as content is no longer present by default at
the
> > parent
> > > > folders.
> > > > 
> > > > I don't understand what you mean.
> > > > 
> > > > The previous policy was the components can't depend on content, and this
is
> > what
> > > > this DEPS line is supposed to enforce.
> > > 
> > > You can only remove a dep that has been added before(inherited from parent
> > folder).
> > > content has been removed from the parent folder and this removal is a
stale
> > value and must be removed.
> > > 
> > > Same with net/base/mock_host_resolver which is no longer being used here.
> > 
> > So this has been broken for awhile? Sorry for the questions, just trying to
> > understand this.
> 
> Not at all, happy to answer. Yes this is an unrequired value since awhile like
the other one from net which wasn't updated.
> You can see a similar change here https://codereview.chromium.org/25727006
> 
> Basically without //content being added at a top level directory, there is no
reason to remove it here.

OK, thanks. Just wanted to confirm.

//components/invalidation LGTM

[pavely, 2015-06-25 21:30:13]

lgtm

[sky, 2015-06-25 22:26:08]

Rubber stamp LGTM

[Ryan Sleevi, 2015-06-26 01:39:40]

lgtm

[Bernhard Bauer, 2015-06-26 14:13:15]

lgtm

[Roger Tawa OOO till Jul 10th, 2015-06-26 14:51:45]

lgtm for components/invalidation/impl/DEPS

[sdefresne, 2015-06-26 16:00:05]

https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
File components/invalidation/DEPS (left):

https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
components/invalidation/DEPS:2: "-content",
Please keep this dependency as the invalidation component should not depends on
content (as it is used on iOS).

https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
File components/invalidation/impl/DEPS (left):

https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
components/invalidation/impl/DEPS:2: "-content",
Keep this line for the same reason.

[Nicolas Zea, 2015-06-26 16:08:46]

rs LGTM

[knn, 2015-06-26 16:37:59]

On 2015/06/26 16:00:05, sdefresne wrote:
>
https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
> File components/invalidation/DEPS (left):
> 
>
https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
> components/invalidation/DEPS:2: "-content",
> Please keep this dependency as the invalidation component should not depends
on
> content (as it is used on iOS).
> 
>
https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
> File components/invalidation/impl/DEPS (left):
> 
>
https://codereview.chromium.org/1191393008/diff/180001/components/invalidatio...
> components/invalidation/impl/DEPS:2: "-content",
> Keep this line for the same reason.

Discussed offline with sdefresne@ Readding "-content" to deter future additions.

[sdefresne, 2015-06-26 16:40:43]

lgtm

[knn, 2015-06-26 16:44:36]

The CQ bit was checked by knn@chromium.org

[knn, 2015-06-26 16:44:37]

The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org,
rsleevi@chromium.org, erg@chromium.org, dcheng@chromium.org, sky@chromium.org,
bauerb@chromium.org, rogerta@chromium.org, zea@chromium.org
Link to the patchset: https://codereview.chromium.org/1191393008/#ps200001
(title: "Explicitly forbid content to prevent future additions")

[commit-bot: I haz the power, 2015-06-26 16:45:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1191393008/200001

[commit-bot: I haz the power, 2015-06-26 18:19:15]

Committed patchset #10 (id:200001)

[commit-bot: I haz the power, 2015-06-26 18:20:16]

Patchset 10 (id:??) landed as
https://crrev.com/062cdbbade5d2c804bce7ff7b320d36cb97c39da
Cr-Commit-Position: refs/heads/master@{#336413}

[knn, 2016-09-12 22:16:47]

Patchset #1 (id:1) has been deleted