Create top level apps component and move some apps code there.

Create top level apps component and move some apps code there.

This is a small patch to begin the process of pulling apps code out of chrome and the extensions system.

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

[benwells, 2013-01-11 04:42:06]

Yoyo - before I publish more widely, how does this look to you?

[benwells, 2013-01-11 04:52:34]

https://codereview.chromium.org/11829055/diff/4001/apps/DEPS
File apps/DEPS (right):

https://codereview.chromium.org/11829055/diff/4001/apps/DEPS#newcode8
apps/DEPS:8: "+chrome/common/chrome_notification_types.h",
I could remove this dependency if we have
chrome/common/extensions/extension_notification_types.h and move the
APP_TERMINATING notification back to content.

[Yoyo Zhou, 2013-01-14 23:37:02]

LGTM I think.

https://codereview.chromium.org/11829055/diff/4001/apps/DEPS
File apps/DEPS (right):

https://codereview.chromium.org/11829055/diff/4001/apps/DEPS#newcode8
apps/DEPS:8: "+chrome/common/chrome_notification_types.h",
On 2013/01/11 04:52:34, benwells wrote:
> I could remove this dependency if we have
> chrome/common/extensions/extension_notification_types.h and move the
> APP_TERMINATING notification back to content.

May not be worth it, as I think there's a long-term goal to use
NotificationObservers less.

https://codereview.chromium.org/11829055/diff/4001/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/11829055/diff/4001/apps/apps.gypi#newcode1
apps/apps.gypi:1: # Copyright (c) 2012 The Chromium Authors. All rights
reserved.
nit: 2013

https://codereview.chromium.org/11829055/diff/4001/apps/apps.gypi#newcode25
apps/apps.gypi:25: # All .cc, .h, .m, and .mm files under browser/extensions
except for
I don't understand this comment.

[benwells, 2013-01-15 03:15:46]

+ben
+miket

Ben / Mike - I'll send a doc with some background.

https://codereview.chromium.org/11829055/diff/4001/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/11829055/diff/4001/apps/apps.gypi#newcode1
apps/apps.gypi:1: # Copyright (c) 2012 The Chromium Authors. All rights
reserved.
On 2013/01/14 23:37:02, Yoyo Zhou wrote:
> nit: 2013

Done.

https://codereview.chromium.org/11829055/diff/4001/apps/apps.gypi#newcode25
apps/apps.gypi:25: # All .cc, .h, .m, and .mm files under browser/extensions
except for
On 2013/01/14 23:37:02, Yoyo Zhou wrote:
> I don't understand this comment.

Comment gone.

[Ben Goodger (Google), 2013-01-15 22:41:09]

lgtm

https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi#newcode9
apps/apps.gypi:9: 'type': 'static_library',
the intent is for this to become a <(component) eventually, right?

[benwells, 2013-01-15 23:10:38]

On 2013/01/15 22:41:09, Ben Goodger (Google) wrote:
> lgtm
> 
> https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi
> File apps/apps.gypi (right):
> 
> https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi#newcode9
> apps/apps.gypi:9: 'type': 'static_library',
> the intent is for this to become a <(component) eventually, right?

I hadn't thought about it and don't know a lot about it.

I assume <(component) maps to a dll or static library depending on the build
type. Does it have any other effect? What are the advantages of it being a
component apart from build time in component builds?

[Ben Goodger (Google), 2013-01-15 23:14:03]

For one thing, it forces your dependencies to be well resolved. I don't
think that's the case for you right now.

-Ben


On Tue, Jan 15, 2013 at 3:10 PM, <benwells@chromium.org> wrote:

> On 2013/01/15 22:41:09, Ben Goodger (Google) wrote:
>
>> lgtm
>>
>
> 
https://codereview.chromium.**org/11829055/diff/15001/apps/**apps.gypi<https:...
>> File apps/apps.gypi (right):
>>
>
>  https://codereview.chromium.**org/11829055/diff/15001/apps/**
>>
apps.gypi#newcode9<https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi#newcode9>
>> apps/apps.gypi:9: 'type': 'static_library',
>> the intent is for this to become a <(component) eventually, right?
>>
>
> I hadn't thought about it and don't know a lot about it.
>
> I assume <(component) maps to a dll or static library depending on the
> build
> type. Does it have any other effect? What are the advantages of it being a
> component apart from build time in component builds?
>
>
https://codereview.chromium.**org/11829055/<https://codereview.chromium.org/1...
>

[benwells, 2013-01-16 00:47:18]

On 2013/01/15 23:14:03, Ben Goodger (Google) wrote:
> For one thing, it forces your dependencies to be well resolved. I don't
> think that's the case for you right now.

I just had a play with it and yep, it looks like making it a component would
force us to get rid of circular dependencies between chrome/browser and apps.
That is the goal, and having something enforce that would be excellent, so yes,
moving to a <(component) is the goal :)
> 
> -Ben
> 
> 
> On Tue, Jan 15, 2013 at 3:10 PM, <mailto:benwells@chromium.org> wrote:
> 
> > On 2013/01/15 22:41:09, Ben Goodger (Google) wrote:
> >
> >> lgtm
> >>
> >
> > 
>
https://codereview.chromium.**org/11829055/diff/15001/apps/**apps.gypi%3Chttp...>
> >> File apps/apps.gypi (right):
> >>
> >
> >  https://codereview.chromium.**org/11829055/diff/15001/apps/**
> >>
>
apps.gypi#newcode9<https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi#newcode9>
> >> apps/apps.gypi:9: 'type': 'static_library',
> >> the intent is for this to become a <(component) eventually, right?
> >>
> >
> > I hadn't thought about it and don't know a lot about it.
> >
> > I assume <(component) maps to a dll or static library depending on the
> > build
> > type. Does it have any other effect? What are the advantages of it being a
> > component apart from build time in component builds?
> >
> >
>
https://codereview.chromium.**org/11829055/%3Chttps://codereview.chromium.org...>
> >

[Ben Goodger (Google), 2013-01-16 17:57:29]

cool :-)


On Tue, Jan 15, 2013 at 4:47 PM, <benwells@chromium.org> wrote:

> On 2013/01/15 23:14:03, Ben Goodger (Google) wrote:
>
>> For one thing, it forces your dependencies to be well resolved. I don't
>> think that's the case for you right now.
>>
>
> I just had a play with it and yep, it looks like making it a component
> would
> force us to get rid of circular dependencies between chrome/browser and
> apps.
> That is the goal, and having something enforce that would be excellent, so
> yes,
> moving to a <(component) is the goal :)
>
>  -Ben
>>
>
>
>  On Tue, Jan 15, 2013 at 3:10 PM, <mailto:benwells@chromium.org> wrote:
>>
>
>  > On 2013/01/15 22:41:09, Ben Goodger (Google) wrote:
>> >
>> >> lgtm
>> >>
>> >
>> >
>>
>
> https://codereview.chromium.****org/11829055/diff/15001/apps/***
> *apps.gypi%3Chttps://coderevie**w.chromium.org/11829055/diff/**
>
15001/apps/apps.gypi<http://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi>
> >
>
>> >> File apps/apps.gypi (right):
>> >>
>> >
>> >  https://codereview.chromium.****org/11829055/diff/15001/apps/****
>> >>
>>
>
> apps.gypi#newcode9<https://**codereview.chromium.org/**
>
11829055/diff/15001/apps/apps.**gypi#newcode9<https://codereview.chromium.org/11829055/diff/15001/apps/apps.gypi#newcode9>
> >
>
>  >> apps/apps.gypi:9: 'type': 'static_library',
>> >> the intent is for this to become a <(component) eventually, right?
>> >>
>> >
>> > I hadn't thought about it and don't know a lot about it.
>> >
>> > I assume <(component) maps to a dll or static library depending on the
>> > build
>> > type. Does it have any other effect? What are the advantages of it
>> being a
>> > component apart from build time in component builds?
>> >
>> >
>>
>
> https://codereview.chromium.****org/11829055/%3Chttps://codere**
> view.chromium.org/11829055/ <http://codereview.chromium.org/11829055/>>
>
>> >
>>
>
>
>
>
https://codereview.chromium.**org/11829055/<https://codereview.chromium.org/1...
>

[miket_OOO, 2013-01-16 21:53:51]

lgtm

https://codereview.chromium.org/11829055/diff/15001/chrome/browser/extensions...
File chrome/browser/extensions/DEPS (right):

https://codereview.chromium.org/11829055/diff/15001/chrome/browser/extensions...
chrome/browser/extensions/DEPS:2: "-apps",
Is this a --similarity problem during upload?

[benwells, 2013-01-16 23:21:00]

https://codereview.chromium.org/11829055/diff/15001/chrome/browser/extensions...
File chrome/browser/extensions/DEPS (right):

https://codereview.chromium.org/11829055/diff/15001/chrome/browser/extensions...
chrome/browser/extensions/DEPS:2: "-apps",
On 2013/01/16 21:53:51, miket wrote:
> Is this a --similarity problem during upload?

I guess so. This is a new file which it thinks is a copy and modify of some
other file.

[tapted, 2013-01-16 23:56:38]

https://codereview.chromium.org/11829055/diff/15001/apps/app_restore_service_...
File apps/app_restore_service_browsertest.cc (right):

https://codereview.chromium.org/11829055/diff/15001/apps/app_restore_service_...
apps/app_restore_service_browsertest.cc:1: // Copyright (c) 2012 The Chromium
Authors. All rights reserved.
nit: should this be 2013, too?

https://codereview.chromium.org/11829055/diff/15001/apps/app_restore_service_...
apps/app_restore_service_browsertest.cc:55: }  // apps
nit: missing 'namespace' in comment

[benwells, 2013-01-21 11:12:18]

https://codereview.chromium.org/11829055/diff/15001/apps/app_restore_service_...
File apps/app_restore_service_browsertest.cc (right):

https://codereview.chromium.org/11829055/diff/15001/apps/app_restore_service_...
apps/app_restore_service_browsertest.cc:1: // Copyright (c) 2012 The Chromium
Authors. All rights reserved.
On 2013/01/16 23:56:38, tapted wrote:
> nit: should this be 2013, too?

Done.

https://codereview.chromium.org/11829055/diff/15001/apps/app_restore_service_...
apps/app_restore_service_browsertest.cc:55: }  // apps
On 2013/01/16 23:56:38, tapted wrote:
> nit: missing 'namespace' in comment

Done.

[commit-bot: I haz the power, 2013-01-21 23:21:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/11829055/35001

[commit-bot: I haz the power, 2013-01-22 02:23:31]

Change committed as 177968