Move Chrome terminating notification out of //apps

Move Chrome terminating notification out of //apps

Part of removing the chrome dependency from //apps. Make the embedder (chrome)
extension system notify the app restore service when it will terminate, instead
of relying on a Chrome notification via NotificationService. This builds on
https://codereview.chromium.org/425303002 to some extent.

It would be nicer not to use the Chrome notification at all, but I'm not sure
how to do that -- at least this way, the responsibility is in c/b/extensions
rather than elsewhere in Chrome.

R=benwells@chromium.org
BUG=679971
Review-Url: https://codereview.chromium.org/2623273011
Cr-Commit-Position: refs/heads/master@{#443643}
Committed: https://chromium.googlesource.com/chromium/src/+/2fc6af94465bf4c86d723bab970f37f29ee3f944

[michaelpg, 2017-01-13 03:32:38]

Description was changed from

==========
Move Chrome terminating notification out of //apps

Part of removing the chrome dependency from //apps. Make the embedder (chrome)
extension system notify the app restore service when it will terminate, instead
of relying on a Chrome notification via NotificationService. This builds on
https://codereview.chromium.org/425303002 to some extent.

It would be nicer not to use the Chrome notification at all, but I'm not sure
how to do that -- at least this way, the responsibility is in c/b/extensions
rather than elsewhere in Chrome.

R=benwells@chromium.org
BUG=679971
==========

to

==========
Move Chrome terminating notification out of //apps

Part of removing the chrome dependency from //apps. Make the embedder (chrome)
extension system notify the app restore service when it will terminate, instead
of relying on a Chrome notification via NotificationService. This builds on
https://codereview.chromium.org/425303002 to some extent.

It would be nicer not to use the Chrome notification at all, but I'm not sure
how to do that -- at least this way, the responsibility is in c/b/extensions
rather than elsewhere in Chrome.

R=benwells@chromium.org
BUG=679971
==========

[michaelpg, 2017-01-13 03:40:21]

Ben: It's unclear to me how Chrome should use /apps. I guess there are two
goals: make /apps a component that uses //extensions (but not vice versa), and
make /apps a component that doesn't use /chrome.

https://crbug.com/159366 started out saying we don't want
chrome/browser/extensions to depend on /apps, and there's the "-apps" rule in
chrome/browser/extensions/DEPS you added. But that has a confusing comment:

  # TODO(benwells): Once the extensions component is established
  # and there are only chrome specific extension things left in
  # chrome/browser/extensions, the restriction of not being able
  # to depend on apps will be lifted.

There aren't *only* Chrome-specific extension things in
chrome/browser/extensions, but I'm in the process of moving the
non-Chrome-specific things out. What is the purpose of this rule?

This later comment suggests that chrome-specific apps code should go in
chrome/browser/apps. There's very little in that directory. Is that where you'd
prefer an application (chrome) lifetime observer to go to notify the platform
app lifetime monitor?

[benwells, 2017-01-13 03:50:53]

lgtm

I can't remember what I was thinking when writing that comment, sorry. Maybe
that's not a biggie though ... the goals may have changed slightly now.

One thing I am interested in us doing is making it so we can build chrome
without any apps support. When only chromeos has support for packaged apps, this
is something we'll want to do. Maybe that is something we can use to drive the
refactorings you're doing.

[benwells, 2017-01-13 03:50:53]

lgtm

I can't remember what I was thinking when writing that comment, sorry. Maybe
that's not a biggie though ... the goals may have changed slightly now.

One thing I am interested in us doing is making it so we can build chrome
without any apps support. When only chromeos has support for packaged apps, this
is something we'll want to do. Maybe that is something we can use to drive the
refactorings you're doing.

[benwells, 2017-01-13 03:50:57]

lgtm

I can't remember what I was thinking when writing that comment, sorry. Maybe
that's not a biggie though ... the goals may have changed slightly now.

One thing I am interested in us doing is making it so we can build chrome
without any apps support. When only chromeos has support for packaged apps, this
is something we'll want to do. Maybe that is something we can use to drive the
refactorings you're doing.

[benwells, 2017-01-13 03:50:57]

lgtm

I can't remember what I was thinking when writing that comment, sorry. Maybe
that's not a biggie though ... the goals may have changed slightly now.

One thing I am interested in us doing is making it so we can build chrome
without any apps support. When only chromeos has support for packaged apps, this
is something we'll want to do. Maybe that is something we can use to drive the
refactorings you're doing.

[benwells, 2017-01-13 03:50:58]

lgtm

I can't remember what I was thinking when writing that comment, sorry. Maybe
that's not a biggie though ... the goals may have changed slightly now.

One thing I am interested in us doing is making it so we can build chrome
without any apps support. When only chromeos has support for packaged apps, this
is something we'll want to do. Maybe that is something we can use to drive the
refactorings you're doing.

[michaelpg, 2017-01-13 18:55:04]

On 2017/01/13 03:50:58, benwells wrote:
> lgtm
> 
> I can't remember what I was thinking when writing that comment, sorry. Maybe
> that's not a biggie though ... the goals may have changed slightly now.

OK, thanks. I've removed the apps restrictions from
chrome/browser/extensions/DEPS.

> 
> One thing I am interested in us doing is making it so we can build chrome
> without any apps support. When only chromeos has support for packaged apps,
this
> is something we'll want to do. Maybe that is something we can use to drive the
> refactorings you're doing.

Yes, that's definitely one goal of my app_shell work. Certainly removing apps
APIs
will be a big win when they're not needed. But I'm not sure it's worth the work
to
explicitly make all extensions code independent of //apps, since that's a small
amount of code anyway. I can live with a few ENABLE_APPS #ifdef's in chrome if
that's where we end up.

[michaelpg, 2017-01-13 18:55:17]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2017-01-13 18:55:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org
Link to the patchset: https://codereview.chromium.org/2623273011/#ps20001
(title: "remove DEPS rule")

[commit-bot: I haz the power, 2017-01-13 18:56:09]

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-01-13 19:54:22]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484333717657960,
"parent_rev": "4480968bdebb5927eeda1acdc5a25f8d6cdfe357", "commit_rev":
"2fc6af94465bf4c86d723bab970f37f29ee3f944"}

[commit-bot: I haz the power, 2017-01-13 19:54:53]

Description was changed from

==========
Move Chrome terminating notification out of //apps

Part of removing the chrome dependency from //apps. Make the embedder (chrome)
extension system notify the app restore service when it will terminate, instead
of relying on a Chrome notification via NotificationService. This builds on
https://codereview.chromium.org/425303002 to some extent.

It would be nicer not to use the Chrome notification at all, but I'm not sure
how to do that -- at least this way, the responsibility is in c/b/extensions
rather than elsewhere in Chrome.

R=benwells@chromium.org
BUG=679971
==========

to

==========
Move Chrome terminating notification out of //apps

Part of removing the chrome dependency from //apps. Make the embedder (chrome)
extension system notify the app restore service when it will terminate, instead
of relying on a Chrome notification via NotificationService. This builds on
https://codereview.chromium.org/425303002 to some extent.

It would be nicer not to use the Chrome notification at all, but I'm not sure
how to do that -- at least this way, the responsibility is in c/b/extensions
rather than elsewhere in Chrome.

R=benwells@chromium.org
BUG=679971

Review-Url: https://codereview.chromium.org/2623273011
Cr-Commit-Position: refs/heads/master@{#443643}
Committed:
https://chromium.googlesource.com/chromium/src/+/2fc6af94465bf4c86d723bab970f...
==========

[commit-bot: I haz the power, 2017-01-13 19:54:55]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2fc6af94465bf4c86d723bab970f...