Add components/chrome_apps

Add components/chrome_apps

This is a new component that will contain Chrome component apps.

Add empty implementation for a new webstore_widget app, which will be used
to show Chrome Webstore Gallery widget to the user (wich displays filtered
list of Chrome Webstore apps and from which the user will be able to
install apps).
The widget is currently implemented as part of file manager app, but will be moved
to a separate app to support more general use cases.

NOTE: I expect to have more component apps moved to the new components
module in the future, first one being file manager.

BUG=477106
TEST=compiles

Committed: https://crrev.com/56ebc035717d90057a245bfc7c7e847ea97f938f
Cr-Commit-Position: refs/heads/master@{#327949}

[tbarzic, 2015-04-28 03:18:16]

tbarzic@chromium.org changed reviewers:
+ asargent@chromium.org, blundell@chromium.org, fukino@chromium.org,
sky@chromium.org, yoshiki@chromium.org

[tbarzic, 2015-04-28 03:18:16]

blundell: owner for components
asargent: owner for extensions
yoshiki: for new component code

fukino, sky: FYI

Please let me know if you have a good idea for additional owners in
components/component_apps. or maybe a better name for the new dir
Thanks

[blundell, 2015-04-28 08:08:50]

Why wouldn't each component app just be its own component?

[tbarzic, 2015-04-28 16:50:34]

On 2015/04/28 08:08:50, blundell wrote:
> Why wouldn't each component app just be its own component?

so we don't have to setup app resource mapping and add resource entries in
chrome/browser/extensions/chrome_component_extension_resource_manager.cc for
each individual app.
Also (in case we end up moving existing apps from chrome/browser/resources), few
apps have only their manifest in Chrome repo, so IMHO, it would be a bit too
much to have a separate component only containing a manifest.json

[asargent_no_longer_on_chrome, 2015-04-28 19:40:09]

nit: typo in CL description "This is new component that will contain Chrome
component apps."

Perhaps you meant "This is a new ..."

otherwise lgtm

I agree that it makes sense to have one top-level component_apps directory with
subdirs instead of one-per-app to cut down on boilerplate gyp changes, etc.

https://codereview.chromium.org/1103943003/diff/80001/components/component_ap...
File components/component_apps/webstore_widget/app/background.js (right):

https://codereview.chromium.org/1103943003/diff/80001/components/component_ap...
components/component_apps/webstore_widget/app/background.js:3: });
did you mean to leave this debugging code in the CL?

[tbarzic, 2015-04-28 19:50:28]

https://codereview.chromium.org/1103943003/diff/80001/components/component_ap...
File components/component_apps/webstore_widget/app/background.js (right):

https://codereview.chromium.org/1103943003/diff/80001/components/component_ap...
components/component_apps/webstore_widget/app/background.js:3: });
On 2015/04/28 19:40:09, Antony Sargent wrote:
> did you mean to leave this debugging code in the CL?

Not really.. removed

[blundell, 2015-04-29 12:31:49]

What is the concrete motivation for having these apps live in //components?
They'll be used by something other than //chrome, presumably? Will that
something potentially want a different set of apps to be available to the user
than //chrome will? If so, would this structure naturally enable that?

Assuming that it does make the most sense for all of these to live in the same
component (it feels not in the spirit of //components to me but I am unfamiliar
with the apps side of things), I would call it //components/chrome_apps (all of
the things in here will be Chrome apps, right?).

[tbarzic, 2015-04-29 19:48:06]

On 2015/04/29 12:31:49, blundell wrote:
> What is the concrete motivation for having these apps live in //components?
> They'll be used by something other than //chrome, presumably? Will that
> something potentially want a different set of apps to be available to the user
> than //chrome will? If so, would this structure naturally enable that?
> 
> Assuming that it does make the most sense for all of these to live in the same
> component (it feels not in the spirit of //components to me but I am
unfamiliar
> with the apps side of things), I would call it //components/chrome_apps (all
of
> the things in here will be Chrome apps, right?).

So, the main motivation was to add the new (webstore_widget) app in a location
where it can share code with file manager app (in //ui/file_manager).
I initially planned to add the app to //ui, but //ui owners felt that neither of
these apps really belong in //ui, and we ended up concluding that they can be
moved to //components/apps (I'll cc you on the discussion thread).

Most of component apps currently live under chrome/browser/resources (as did
file manager), but that seems a bit weird given that they are really a
standalone apps tightly bundled with Chrome (and with some special privileges),
rather than part of chrome implementation (and this was one of the reasons for
moving file_manager code out); so it would probably make sense to move others
too.

Even if only file_manager and webstore_widget app were moved to //components, it
makes sense to keep them together in order to reduce boilerplate code (setting
up app resource maps); though in that case the case for having separate
components for the two could be made a bit stronger.

Antony, WDYT, does it make sense to you to move other apps out of
//chrome/browser/resources?

[blundell, 2015-04-30 14:31:43]

lgtm reluctantly :P

Components are supposed to be pieces of functionality that are reused by
multiple embedders. In this case it seems like //components is being used
essentially as a dumping ground because neither //chromeos OWNERS nor //ui
OWNERS want to have this code in those modules. I would vote for //chromeos/ui
myself, but there's no point in beating a dead horse.

[asargent_no_longer_on_chrome, 2015-04-30 17:33:49]

> Antony, WDYT, does it make sense to you to move other apps out of
> //chrome/browser/resources?

I think it would be nice to have them all in one place, perhaps with the use of
a subdirectory to distinguish between ones that are applicable for any instance
of chrome and those that are chromeos only. I don't really have any strong
opinion about *where* that is (eg in //chrome/browser/resources, //ui,
//components, or some other top-level directory like //chrome-apps).

I do have some sympathy with the concern that //components is meant for reusable
pieces that are built into multiple products or binaries, and so feels a little
weird to have some stuff in there which only works in chrome/chromeos. But if
long-term we want these to be able to work in something like app_shell or
whatever, at that point they aren't chrome/chromeos specific anymore. Again
though, I really don't have much of a preference and trust you to do what's
sensible.

[yoshiki, 2015-05-01 07:55:03]

new component code LGTM with tiny nits.

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
File components/chrome_apps/webstore_widget/app/background.js (right):

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
components/chrome_apps/webstore_widget/app/background.js:1: // TODO(tbarzic):
Implement this.
nit: I think it's better to add a copy write header.

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
File components/chrome_apps/webstore_widget/manifest.json (right):

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
components/chrome_apps/webstore_widget/manifest.json:2: "key":
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0ZSmlOjtKoapSOZ4kH4vpSh2WzGxT7AlJgTet+0v5O/eV56dg1D+hpIltMswe47zX3kozOOzZ3LhnLHZb7kBIyVdgmtAv73XP1/CjvO7w1F5dzdS3zqJEg4JIblKmGIs8Zb93rols5DsJth58w3nPDz94clWQY3VI0IsgPfgssi/5OHyyzd3fVZS6UQU1XBkfZ+mpUQSKD293Rqq2aol1WgmyVumj7o8YM73SU/9Hnxla56ew88Zv/A78o1i1X6mcQcLCexDIAwcerJ09TvrLRv0sexRKBx47YZqVmR5yJRNGQiJdV54Es/jP6K8Oy8oZnz7Wea/nAL0b0OKAcq7+QIDAQAB",
nit: Please write the extension ID in comment for reference. It should be
useful.

(eg.
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi...)

[tbarzic, 2015-05-01 17:27:24]

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
File components/chrome_apps/webstore_widget/app/background.js (right):

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
components/chrome_apps/webstore_widget/app/background.js:1: // TODO(tbarzic):
Implement this.
On 2015/05/01 07:55:03, yoshiki wrote:
> nit: I think it's better to add a copy write header.

Done.

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
File components/chrome_apps/webstore_widget/manifest.json (right):

https://codereview.chromium.org/1103943003/diff/120001/components/chrome_apps...
components/chrome_apps/webstore_widget/manifest.json:2: "key":
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0ZSmlOjtKoapSOZ4kH4vpSh2WzGxT7AlJgTet+0v5O/eV56dg1D+hpIltMswe47zX3kozOOzZ3LhnLHZb7kBIyVdgmtAv73XP1/CjvO7w1F5dzdS3zqJEg4JIblKmGIs8Zb93rols5DsJth58w3nPDz94clWQY3VI0IsgPfgssi/5OHyyzd3fVZS6UQU1XBkfZ+mpUQSKD293Rqq2aol1WgmyVumj7o8YM73SU/9Hnxla56ew88Zv/A78o1i1X6mcQcLCexDIAwcerJ09TvrLRv0sexRKBx47YZqVmR5yJRNGQiJdV54Es/jP6K8Oy8oZnz7Wea/nAL0b0OKAcq7+QIDAQAB",
On 2015/05/01 07:55:03, yoshiki wrote:
> nit: Please write the extension ID in comment for reference. It should be
> useful.
> 
> (eg.
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi...)

Done.

[tbarzic, 2015-05-01 17:27:37]

The CQ bit was checked by tbarzic@chromium.org

[tbarzic, 2015-05-01 17:27:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org, yoshiki@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1103943003/#ps140001
(title: " ")

[commit-bot: I haz the power, 2015-05-01 17:28:25]

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

[commit-bot: I haz the power, 2015-05-01 18:44:43]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-05-01 18:46:31]

Patchset 8 (id:??) landed as
https://crrev.com/56ebc035717d90057a245bfc7c7e847ea97f938f
Cr-Commit-Position: refs/heads/master@{#327949}