Add a ServiceWorkerContextObserver to content's public API.

Add a ServiceWorkerContextObserver to content's public API.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for
crrev.com/2942513002, where the method is used to check if a
manifest URL lies within the scope of a service worker.

BUG=625051
Review-Url: https://codereview.chromium.org/2934163003
Cr-Commit-Position: refs/heads/master@{#479568}
Committed: https://chromium.googlesource.com/chromium/src/+/5bce7f41747fd8e3918ae354977e655487237e4c

[dominickn, 2017-06-14 02:00:07]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 02:00:40]

Dry run: CQ is trying da patch.

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

[dominickn, 2017-06-14 02:41:14]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 02:41:33]

Dry run: CQ is trying da patch.

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

[dominickn, 2017-06-14 03:22:58]

Description was changed from

==========
Add a ServiceWorkerContextObserver to content/public.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for the first public
observer, which will need to check whether or not a URL lies within the
scope of a service worker.

BUG=625051
==========

to

==========
Add a ServiceWorkerContextObserver to content's public API.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for the first public
observer, which will need to check whether or not a URL lies within the
scope of a service worker.

BUG=625051
==========

[dominickn, 2017-06-14 03:25:05]

Description was changed from

==========
Add a ServiceWorkerContextObserver to content's public API.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for the first public
observer, which will need to check whether or not a URL lies within the
scope of a service worker.

BUG=625051
==========

to

==========
Add a ServiceWorkerContextObserver to content's public API.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for
crrev.com/2942513002, where the method is used to check if a
manifest URL lies within the scope of a service worker.

BUG=625051
==========

[dominickn, 2017-06-14 03:25:05]

dominickn@chromium.org changed reviewers:
+ kinuko@chromium.org

[dominickn, 2017-06-14 03:25:36]

PTAL, thanks! The follow up CL that uses the public observer is
https://codereview.chromium.org/2942513002/

[commit-bot: I haz the power, 2017-06-14 04:51:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-14 04:51:21]

Dry run: This issue passed the CQ dry run.

[kinuko, 2017-06-14 07:33:45]

lgtm

https://codereview.chromium.org/2934163003/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/2934163003/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_context_wrapper.cc:140:
core_observer_list_->RemoveObserver(this);
I think this wrapper almost always live longer than core... while this is
probably fine as well.  (Can you comment about that not to confuse readers?)

[dominickn, 2017-06-14 09:10:58]

Thanks!

https://codereview.chromium.org/2934163003/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/2934163003/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_context_wrapper.cc:140:
core_observer_list_->RemoveObserver(this);
On 2017/06/14 07:33:45, kinuko wrote:
> I think this wrapper almost always live longer than core... while this is
> probably fine as well.  (Can you comment about that not to confuse readers?)

You can see in #ps1 that a bunch of test failures occurred when I didn't have
this RemoveObserver call. There were a bunch of use-after-frees on the Wrapper
object, I think stemming from the EmbeddedWorkerTestHelper and how it
creates/destroys a Wrapper and a ContextCore. I added a clarifying comment for
this.

[dominickn, 2017-06-14 09:11:12]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 09:11:31]

Dry run: CQ is trying da patch.

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

[kinuko, 2017-06-14 09:30:02]

On 2017/06/14 09:10:58, dominickn wrote:
> Thanks!
> 
>
https://codereview.chromium.org/2934163003/diff/20001/content/browser/service...
> File content/browser/service_worker/service_worker_context_wrapper.cc (right):
> 
>
https://codereview.chromium.org/2934163003/diff/20001/content/browser/service...
> content/browser/service_worker/service_worker_context_wrapper.cc:140:
> core_observer_list_->RemoveObserver(this);
> On 2017/06/14 07:33:45, kinuko wrote:
> > I think this wrapper almost always live longer than core... while this is
> > probably fine as well.  (Can you comment about that not to confuse readers?)
> 
> You can see in #ps1 that a bunch of test failures occurred when I didn't have
> this RemoveObserver call. There were a bunch of use-after-frees on the Wrapper
> object, I think stemming from the EmbeddedWorkerTestHelper and how it
> creates/destroys a Wrapper and a ContextCore. I added a clarifying comment for
> this.

I see, tests may not be plumbed correctly. Thanks for the comment!

[commit-bot: I haz the power, 2017-06-14 13:16:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-14 13:16:17]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dominickn, 2017-06-15 00:16:09]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2017-06-15 00:16:09]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org
Link to the patchset: https://codereview.chromium.org/2934163003/#ps40001
(title: "Clarifying comment")

[commit-bot: I haz the power, 2017-06-15 00:16:20]

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-06-15 01:27:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497485769364900,
"parent_rev": "b27799f5b77b176915bec9228003524f60ed589e", "commit_rev":
"5bce7f41747fd8e3918ae354977e655487237e4c"}

[commit-bot: I haz the power, 2017-06-15 01:27:28]

Description was changed from

==========
Add a ServiceWorkerContextObserver to content's public API.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for
crrev.com/2942513002, where the method is used to check if a
manifest URL lies within the scope of a service worker.

BUG=625051
==========

to

==========
Add a ServiceWorkerContextObserver to content's public API.

This CL introduces a ServiceWorkerContextObserver which may be used by
classes outside of content. The observer is implemented by proxying the
internal ServiceWorkerContextCoreObserver method calls via
ServiceWorkerContextWrapper.

This CL also exposes ServiceWorkerUtils::ScopeMatches on the public
ServiceWorkerContext interface in preparation for
crrev.com/2942513002, where the method is used to check if a
manifest URL lies within the scope of a service worker.

BUG=625051

Review-Url: https://codereview.chromium.org/2934163003
Cr-Commit-Position: refs/heads/master@{#479568}
Committed:
https://chromium.googlesource.com/chromium/src/+/5bce7f41747fd8e3918ae354977e...
==========

[commit-bot: I haz the power, 2017-06-15 01:27:29]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/5bce7f41747fd8e3918ae354977e...