Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971, 159366
TBR=sky@chromium.org

Review-Url: https://codereview.chromium.org/2789253003
Cr-Commit-Position: refs/heads/master@{#462314}
Committed: https://chromium.googlesource.com/chromium/src/+/dbdcdcc7eb02d34200dca4a21f8528a7332f7d90

[michaelpg, 2017-04-03 22:38:51]

Description was changed from

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

A few Chrome-specific files and functions from the top-level //apps
directory are moved into //c/b/apps.

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971
==========

to

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971
==========

[michaelpg, 2017-04-03 22:38:51]

michaelpg@chromium.org changed reviewers:
+ benwells@chromium.org

[michaelpg, 2017-04-03 22:40:59]

Description was changed from

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971
==========

to

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971,159366
==========

[michaelpg, 2017-04-03 22:42:01]

Patchset #2 (id:20001) has been deleted

[michaelpg, 2017-04-03 22:46:58]

Ben, PTAL - thanks!

[benwells, 2017-04-04 01:14:13]

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/BUI...
File chrome/browser/apps/BUILD.gn (right):

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/BUI...
chrome/browser/apps/BUILD.gn:11: "../apps/app_launch_for_metro_restart_win.cc",
Why do you need the "../apps" on all of these?

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/app...
File chrome/browser/apps/app_load_service.h (right):

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/app...
chrome/browser/apps/app_load_service.h:5: #ifndef
CHROME_BROWSER_APPS_APP_LOAD_SERVICE_H_
Why are you moving this? Is it because it is hard to remove dependencies (e.g.
extension service) from this? Or more that it conceptually belongs in //chrome?

[michaelpg, 2017-04-04 01:49:32]

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/BUI...
File chrome/browser/apps/BUILD.gn (right):

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/BUI...
chrome/browser/apps/BUILD.gn:11: "../apps/app_launch_for_metro_restart_win.cc",
On 2017/04/04 01:14:13, benwells wrote:
> Why do you need the "../apps" on all of these?

Ah. Originally I had this as a source_set in c/b/extensions, so I forgot to
update these. Kind of an accident that this even worked!

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/app...
File chrome/browser/apps/app_load_service.h (right):

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/app...
chrome/browser/apps/app_load_service.h:5: #ifndef
CHROME_BROWSER_APPS_APP_LOAD_SERVICE_H_
On 2017/04/04 01:14:13, benwells wrote:
> Why are you moving this? Is it because it is hard to remove dependencies (e.g.
> extension service) from this? Or more that it conceptually belongs in
//chrome?

ExtensionService is a big one, also UnpackedInstaller (though I'm making good
progress on unpacking that one). This doesn't have to belong in //chrome
conceptually, but the concepts of unloading, reloading, and installing apps are
only applicable in Chrome atm.

I considered making an //apps/chrome directory after talking with Devlin, but
that pattern doesn't appear anywhere else. But maybe something like
//apps/client, with delegates (rather than dependencies) into Chrome, would be a
better final state?

[benwells, 2017-04-04 03:41:40]

lgtm

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/app...
File chrome/browser/apps/app_load_service.h (right):

https://codereview.chromium.org/2789253003/diff/40001/chrome/browser/apps/app...
chrome/browser/apps/app_load_service.h:5: #ifndef
CHROME_BROWSER_APPS_APP_LOAD_SERVICE_H_
On 2017/04/04 01:49:31, michaelpg wrote:
> On 2017/04/04 01:14:13, benwells wrote:
> > Why are you moving this? Is it because it is hard to remove dependencies
(e.g.
> > extension service) from this? Or more that it conceptually belongs in
> //chrome?
> 
> ExtensionService is a big one, also UnpackedInstaller (though I'm making good
> progress on unpacking that one). This doesn't have to belong in //chrome
> conceptually, but the concepts of unloading, reloading, and installing apps
are
> only applicable in Chrome atm.
> 
> I considered making an //apps/chrome directory after talking with Devlin, but
> that pattern doesn't appear anywhere else. But maybe something like
> //apps/client, with delegates (rather than dependencies) into Chrome, would be
a
> better final state?

ok, i was just wondering. I think having this outside chrome in the end makes
sense, but that doesn't have to happen immediately. Long term chrome won't be
supporting apps at all, only ChromeOS, but that's a ways off.

[michaelpg, 2017-04-05 00:27:27]

Description was changed from

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971,159366
==========

to

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971,159366
TBR=sky@chromium.org
==========

[michaelpg, 2017-04-05 00:27:27]

michaelpg@chromium.org changed reviewers:
+ sky@chromium.org

[michaelpg, 2017-04-05 00:27:27]

michaelpg@chromium.org changed required reviewers:
+ benwells@chromium.org

[michaelpg, 2017-04-05 00:30:03]

michaelpg@chromium.org changed reviewers:
+ erg@chromium.org

[michaelpg, 2017-04-05 00:30:03]

michaelpg@chromium.org changed required reviewers:
+ erg@chromium.org

[michaelpg, 2017-04-05 00:31:05]

erg: PTAL for
chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

Added sky as TBR for:
  chrome/browser/background/background_contents_service.cc
  chrome/browser/renderer_context_menu/render_view_context_menu.cc
  chrome/browser/ui/startup/startup_browser_creator.cc

[sky, 2017-04-05 03:10:19]

LGTM

[Elliot Glaysher, 2017-04-05 17:09:42]

lgtm

[michaelpg, 2017-04-06 00:04:09]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2017-04-06 00:04:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org, erg@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2789253003/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-06 00:04:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-06 01:41:00]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491437049648040,
"parent_rev": "92d289e82e3aac87336b3ee2d50e830ae8fbf384", "commit_rev":
"dbdcdcc7eb02d34200dca4a21f8528a7332f7d90"}

[commit-bot: I haz the power, 2017-04-06 01:41:43]

Description was changed from

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971,159366
TBR=sky@chromium.org
==========

to

==========
Add chrome/browser/apps/BUILD.gn and move Chrome-specific code there

This moves references to c/b/apps from their awkward place in
c/b/extensions to a standalone source_set in c/b/apps/BUILD.gn.

Now, c/b/apps explicitly depends on c/b/extensions, and c/b/extensions
need not explicitlty depend on c/b/apps. (Some circular includes are
still required, e.g. for app-specific API calls within c/b/extensions.)

Apart from BUILD.gn files, the structural changes are:
* moving {apps => chrome/browser/apps}/app_load_service.cc
* adding chrome/browser/apps/browser_context_keyed_service_factories.* to
  handle startup/shutdown tasks

The following gn checks now pass:
* //apps (used to fail)
* //chrome/browser/apps (newly added)

The following gn checks used to pass, and still do:
* //chrome/browser/extensions
* //chrome/browser/ui

The following gn checks still fail, but with fewer errors:
* //chrome/browser

BUG=679971,159366
TBR=sky@chromium.org

Review-Url: https://codereview.chromium.org/2789253003
Cr-Commit-Position: refs/heads/master@{#462314}
Committed:
https://chromium.googlesource.com/chromium/src/+/dbdcdcc7eb02d34200dca4a21f85...
==========

[commit-bot: I haz the power, 2017-04-06 01:41:45]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/dbdcdcc7eb02d34200dca4a21f85...