Introduce the iOS consumer API.

Introduce the iOS consumer API.

The purpose of this API is to make the iOS port less vulnerable to bustage
during merges while in the process of upstreaming (see
https://sites.google.com/a/chromium.org/dev/developers/design-documents/layered-components-technical-approach).

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

[stuartmorgan, 2013-05-28 08:43:40]

https://codereview.chromium.org/16023013/diff/1007/ios/consume/README.txt
File ios/consume/README.txt (right):

https://codereview.chromium.org/16023013/diff/1007/ios/consume/README.txt#new...
ios/consume/README.txt:7: an OWNER.
I wonder if we should put a short version of this last part in the OWNERS file
too?

https://codereview.chromium.org/16023013/diff/1007/ios/consume/ios_consume.gyp
File ios/consume/ios_consume.gyp (right):

https://codereview.chromium.org/16023013/diff/1007/ios/consume/ios_consume.gy...
ios/consume/ios_consume.gyp:1: # Copyright (c) 2013 The Chromium Authors. All
rights reserved.
Remove the (c)

https://codereview.chromium.org/16023013/diff/1007/ios/consume/ios_consume.gy...
ios/consume/ios_consume.gyp:10: 'target_name': 'base',
Having two targets with the same name will be really confusing. How about
ios_consume_<foo> as target naming scheme?

https://codereview.chromium.org/16023013/diff/1007/ios/consume/public/DEPS
File ios/consume/public/DEPS (right):

https://codereview.chromium.org/16023013/diff/1007/ios/consume/public/DEPS#ne...
ios/consume/public/DEPS:2: # The public interfaces cannot reference Chromium
code.
Can you add a comment explaining that this list is duplicated from the top-level
DEPS file? There's no way to guarantee it stays in sync, of course, but it at
least makes it easy for someone to check if it still is.

[blundell, 2013-05-28 09:01:14]

Thanks.

https://codereview.chromium.org/16023013/diff/1007/ios/consume/README.txt
File ios/consume/README.txt (right):

https://codereview.chromium.org/16023013/diff/1007/ios/consume/README.txt#new...
ios/consume/README.txt:7: an OWNER.
Good idea, done.

On 2013/05/28 08:43:40, stuartmorgan wrote:
> I wonder if we should put a short version of this last part in the OWNERS file
> too?

https://codereview.chromium.org/16023013/diff/1007/ios/consume/ios_consume.gyp
File ios/consume/ios_consume.gyp (right):

https://codereview.chromium.org/16023013/diff/1007/ios/consume/ios_consume.gy...
ios/consume/ios_consume.gyp:1: # Copyright (c) 2013 The Chromium Authors. All
rights reserved.
On 2013/05/28 08:43:40, stuartmorgan wrote:
> Remove the (c)

Done.

https://codereview.chromium.org/16023013/diff/1007/ios/consume/ios_consume.gy...
ios/consume/ios_consume.gyp:10: 'target_name': 'base',
On 2013/05/28 08:43:40, stuartmorgan wrote:
> Having two targets with the same name will be really confusing. How about
> ios_consume_<foo> as target naming scheme?

Done.

https://codereview.chromium.org/16023013/diff/1007/ios/consume/public/DEPS
File ios/consume/public/DEPS (right):

https://codereview.chromium.org/16023013/diff/1007/ios/consume/public/DEPS#ne...
ios/consume/public/DEPS:2: # The public interfaces cannot reference Chromium
code.
On 2013/05/28 08:43:40, stuartmorgan wrote:
> Can you add a comment explaining that this list is duplicated from the
top-level
> DEPS file? There's no way to guarantee it stays in sync, of course, but it at
> least makes it easy for someone to check if it still is.

Done.

[jam, 2013-05-28 14:38:02]

lgtm (although I'm not an owner for any of these directories)

https://codereview.chromium.org/16023013/diff/12001/ios/consume/OWNERS
File ios/consume/OWNERS (right):

https://codereview.chromium.org/16023013/diff/12001/ios/consume/OWNERS#newcode4
ios/consume/OWNERS:4: stuartmorgan@chromium.org
nit: the note above is redundant with the README. do you really need another
OWNERS file here, or is the top level one enough (i.e. +rohit)

https://codereview.chromium.org/16023013/diff/12001/ios/consume/public/DEPS
File ios/consume/public/DEPS (right):

https://codereview.chromium.org/16023013/diff/12001/ios/consume/public/DEPS#n...
ios/consume/public/DEPS:4: # be kept in sync with <path_to_src>/DEPS.
nit: just src/DEPS

[darin (slow to review), 2013-05-28 17:47:07]

nit: why "consume" and not "consumer"?

[blundell, 2013-05-28 18:24:06]

On 2013/05/28 17:47:07, darin wrote:
> nit: why "consume" and not "consumer"?

I had been thinking that we would call the two halves of the API "consume" and
"embed" (the latter being for interfaces that not-yet-upstreamed code provides
to Chromium code); however, now that you mention it, "consumer" and "provider"
seem strictly better. I'm happy to move to that naming (I don't like "embedder"
much, though).

[darin (slow to review), 2013-05-28 18:31:41]

OK


On Tue, May 28, 2013 at 11:24 AM, <blundell@chromium.org> wrote:

> On 2013/05/28 17:47:07, darin wrote:
>
>> nit: why "consume" and not "consumer"?
>>
>
> I had been thinking that we would call the two halves of the API "consume"
> and
> "embed" (the latter being for interfaces that not-yet-upstreamed code
> provides
> to Chromium code); however, now that you mention it, "consumer" and
> "provider"
> seem strictly better. I'm happy to move to that naming (I don't like
> "embedder"
> much, though).
>
>
https://codereview.chromium.**org/16023013/<https://codereview.chromium.org/1...
>

[blundell, 2013-05-28 20:26:01]

Thanks for the reviews, all comments applied, including s/consume/consumer/g.

https://codereview.chromium.org/16023013/diff/12001/ios/consume/OWNERS
File ios/consume/OWNERS (right):

https://codereview.chromium.org/16023013/diff/12001/ios/consume/OWNERS#newcode4
ios/consume/OWNERS:4: stuartmorgan@chromium.org
On 2013/05/28 14:38:02, jam wrote:
> nit: the note above is redundant with the README. do you really need another
> OWNERS file here, or is the top level one enough (i.e. +rohit)

Done.

https://codereview.chromium.org/16023013/diff/12001/ios/consume/public/DEPS
File ios/consume/public/DEPS (right):

https://codereview.chromium.org/16023013/diff/12001/ios/consume/public/DEPS#n...
ios/consume/public/DEPS:4: # be kept in sync with <path_to_src>/DEPS.
On 2013/05/28 14:38:02, jam wrote:
> nit: just src/DEPS

Done.

[darin (slow to review), 2013-05-28 21:00:09]

https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/...
File ios/consumer/public/base/util.h (right):

https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/...
ios/consumer/public/base/util.h:9: namespace base {
I recommend avoiding nested namespaces.  I especially recommend avoiding using
"base" as a nested namespace.  Having to type ::base to access code in src/base/
kind of sucks.

Also, it seems a bit humorous that the implementation of ::ios::base code
involves calling methods defined in ::base::ios ;-)

How about just using ios as the namespace or ios_consumer?

[blundell, 2013-05-28 21:08:20]

SGTM. I like the idea of having ios/foo be namespaced ios_foo as a convention,
so I used ios_consumer.

On 2013/05/28 21:00:09, darin wrote:
>
https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/...
> File ios/consumer/public/base/util.h (right):
> 
>
https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/...
> ios/consumer/public/base/util.h:9: namespace base {
> I recommend avoiding nested namespaces.  I especially recommend avoiding using
> "base" as a nested namespace.  Having to type ::base to access code in
src/base/
> kind of sucks.
> 
> Also, it seems a bit humorous that the implementation of ::ios::base code
> involves calling methods defined in ::base::ios ;-)
> 
> How about just using ios as the namespace or ios_consumer?

[darin (slow to review), 2013-05-28 21:11:24]

Note, it can get annoying if you have a lot of those ios_ prefixed
namespaces.  Just have a look around src/webkit/ if you want to see what I
mean.

What has worked best is to just use a single top-level namespace (e.g.,
base::, net:: and content:: come to mind).  We do that for content even
though it has distinct branches of its public API.  Instead of
content_browser:: and content_renderer::, we just use content::.  I think
it helps make things simpler.

-Darin


On Tue, May 28, 2013 at 2:08 PM, <blundell@chromium.org> wrote:

> SGTM. I like the idea of having ios/foo be namespaced ios_foo as a
> convention,
> so I used ios_consumer.
>
>
> On 2013/05/28 21:00:09, darin wrote:
>
> https://codereview.chromium.**org/16023013/diff/24001/ios/**
>
consumer/public/base/util.h<https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/util.h>
>
>> File ios/consumer/public/base/util.**h (right):
>>
>
>
> https://codereview.chromium.**org/16023013/diff/24001/ios/**
>
consumer/public/base/util.h#**newcode9<https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/util.h#newcode9>
>
>> ios/consumer/public/base/util.**h:9: namespace base {
>> I recommend avoiding nested namespaces.  I especially recommend avoiding
>> using
>> "base" as a nested namespace.  Having to type ::base to access code in
>>
> src/base/
>
>> kind of sucks.
>>
>
>  Also, it seems a bit humorous that the implementation of ::ios::base code
>> involves calling methods defined in ::base::ios ;-)
>>
>
>  How about just using ios as the namespace or ios_consumer?
>>
>
>
>
>
https://codereview.chromium.**org/16023013/<https://codereview.chromium.org/1...
>

[blundell, 2013-05-28 21:17:28]

Fine with me as well. Done.

On 2013/05/28 21:11:24, darin wrote:
> Note, it can get annoying if you have a lot of those ios_ prefixed
> namespaces.  Just have a look around src/webkit/ if you want to see what I
> mean.
> 
> What has worked best is to just use a single top-level namespace (e.g.,
> base::, net:: and content:: come to mind).  We do that for content even
> though it has distinct branches of its public API.  Instead of
> content_browser:: and content_renderer::, we just use content::.  I think
> it helps make things simpler.
> 
> -Darin
> 
> 
> On Tue, May 28, 2013 at 2:08 PM, <mailto:blundell@chromium.org> wrote:
> 
> > SGTM. I like the idea of having ios/foo be namespaced ios_foo as a
> > convention,
> > so I used ios_consumer.
> >
> >
> > On 2013/05/28 21:00:09, darin wrote:
> >
> > https://codereview.chromium.**org/16023013/diff/24001/ios/**
> >
>
consumer/public/base/util.h<https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/util.h>
> >
> >> File ios/consumer/public/base/util.**h (right):
> >>
> >
> >
> > https://codereview.chromium.**org/16023013/diff/24001/ios/**
> >
>
consumer/public/base/util.h#**newcode9<https://codereview.chromium.org/16023013/diff/24001/ios/consumer/public/base/util.h#newcode9>
> >
> >> ios/consumer/public/base/util.**h:9: namespace base {
> >> I recommend avoiding nested namespaces.  I especially recommend avoiding
> >> using
> >> "base" as a nested namespace.  Having to type ::base to access code in
> >>
> > src/base/
> >
> >> kind of sucks.
> >>
> >
> >  Also, it seems a bit humorous that the implementation of ::ios::base code
> >> involves calling methods defined in ::base::ios ;-)
> >>
> >
> >  How about just using ios as the namespace or ios_consumer?
> >>
> >
> >
> >
> >
>
https://codereview.chromium.**org/16023013/%3Chttps://codereview.chromium.org...>
> >

[darin (slow to review), 2013-05-28 21:29:04]

LGTM

https://codereview.chromium.org/16023013/diff/33002/ios/consumer/public/DEPS
File ios/consumer/public/DEPS (right):

https://codereview.chromium.org/16023013/diff/33002/ios/consumer/public/DEPS#...
ios/consumer/public/DEPS:4: # be kept in sync with src/DEPS.
note for the future:  it seems like it'd be handy for there to be a way to not
inherit rules for cases like this.  that way we avoid this maintenance problem.

[blundell, 2013-05-30 09:25:49]

On 2013/05/28 21:29:04, darin wrote:
> LGTM
> 
> https://codereview.chromium.org/16023013/diff/33002/ios/consumer/public/DEPS
> File ios/consumer/public/DEPS (right):
> 
>
https://codereview.chromium.org/16023013/diff/33002/ios/consumer/public/DEPS#...
> ios/consumer/public/DEPS:4: # be kept in sync with src/DEPS.
> note for the future:  it seems like it'd be handy for there to be a way to not
> inherit rules for cases like this.  that way we avoid this maintenance
problem.

Filed crbug.com/245225 re: not inheriting DEPS rules.

Stuart, do you want to take a final look?

[stuartmorgan, 2013-05-30 10:59:38]

LGTM

[commit-bot: I haz the power, 2013-05-30 11:00:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/16023013/33002

[commit-bot: I haz the power, 2013-05-30 12:34:49]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-30 13:01:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/blundell@chromium.org/16023013/33002

[commit-bot: I haz the power, 2013-05-30 15:09:33]

Change committed as 203144