Pass the v8::Context through the willDestroyServiceWorkerContextOnWorkerThread event.

Pass the v8::Context through the willDestroyServiceWorkerContextOnWorkerThread event.

Currently it only passes a URL, and this isn't enough to uniquely identify the worker.
Update extension's dispatcher.cc to use the context.

BUG=501569, 525965
R=kinuko@chromium.org, thestig@chromium.org

Committed: https://crrev.com/4a0bb926cb1d83fab27b6ad866c04ba481f66268
Cr-Commit-Position: refs/heads/master@{#346424}

[not at google - send to devlin, 2015-08-26 01:17:07]

Blink side of patch is https://codereview.chromium.org/1310843006. Obviously
that will need to go in first.

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
File content/renderer/service_worker/service_worker_context_client.cc (left):

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
content/renderer/service_worker/service_worker_context_client.cc:394:
DCHECK_EQ(script_url_, GURL(url));
I removed this DCHECK because in order to address the TODO it's more convenient
for it to just ignore |url|. I may do this TODO while I'm here, but not in this
set of patches.

[not at google - send to devlin, 2015-08-26 01:19:57]

kalman@chromium.org changed reviewers:
+ kinuko@chromium.org, thestig@chromium.org
- jochen@chromium.org

[not at google - send to devlin, 2015-08-26 01:20:16]

Sorry, jochen is out - kinuko could you look at the content side, thestig the
chrome side?

[kinuko, 2015-08-26 06:13:17]

lgtm

[kinuko, 2015-08-26 09:04:18]

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
File content/renderer/service_worker/service_worker_context_client.cc (right):

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
content/renderer/service_worker/service_worker_context_client.cc:401:
v8::Local<v8::Context> context) {
Looking into https://codereview.chromium.org/1310843006/ when this one's called
this v8 context may not be already available.  If this is used only as a key to
remove the corresponding extension context could we just use a raw pointer of
this instance or of proxy_?

[not at google - send to devlin, 2015-08-26 17:27:38]

Patchset #2 (id:20001) has been deleted

[not at google - send to devlin, 2015-08-26 17:29:03]

I've updated extensions code to actually use this to prove that it works.

I've also updated the blink patch to make it work :-)

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
File content/renderer/service_worker/service_worker_context_client.cc (right):

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
content/renderer/service_worker/service_worker_context_client.cc:401:
v8::Local<v8::Context> context) {
On 2015/08/26 09:04:18, kinuko wrote:
> Looking into https://codereview.chromium.org/1310843006/ when this one's
called
> this v8 context may not be already available.  If this is used only as a key
to
> remove the corresponding extension context could we just use a raw pointer of
> this instance or of proxy_?

A raw pointer of a v8::Context? How would that help (and also I don't think v8
likes that).

[Lei Zhang, 2015-08-26 20:49:57]

chrome/ lgtm

[kinuko, 2015-08-27 00:36:55]

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
File content/renderer/service_worker/service_worker_context_client.cc (right):

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
content/renderer/service_worker/service_worker_context_client.cc:401:
v8::Local<v8::Context> context) {
On 2015/08/26 17:29:02, kalman wrote:
> On 2015/08/26 09:04:18, kinuko wrote:
> > Looking into https://codereview.chromium.org/1310843006/ when this one's
> called
> > this v8 context may not be already available.  If this is used only as a key
> to
> > remove the corresponding extension context could we just use a raw pointer
of
> > this instance or of proxy_?
> 
> A raw pointer of a v8::Context? How would that help (and also I don't think v8
> likes that).

I was thinking about using a pointer to 'this', ServiceWorkerContextClient, or
whatever that can be used to identify this context.

[not at google - send to devlin, 2015-08-27 15:11:50]

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
File content/renderer/service_worker/service_worker_context_client.cc (right):

https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
content/renderer/service_worker/service_worker_context_client.cc:401:
v8::Local<v8::Context> context) {
On 2015/08/27 00:36:55, kinuko wrote:
> On 2015/08/26 17:29:02, kalman wrote:
> > On 2015/08/26 09:04:18, kinuko wrote:
> > > Looking into https://codereview.chromium.org/1310843006/ when this one's
> > called
> > > this v8 context may not be already available.  If this is used only as a
key
> > to
> > > remove the corresponding extension context could we just use a raw pointer
> of
> > > this instance or of proxy_?
> > 
> > A raw pointer of a v8::Context? How would that help (and also I don't think
v8
> > likes that).
> 
> I was thinking about using a pointer to 'this', ServiceWorkerContextClient, or
> whatever that can be used to identify this context.

I would need to move this into content/public for that to work. v8::Context
seems to work fine + is consistent.

[kinuko, 2015-08-27 15:59:26]

On 2015/08/27 15:11:50, kalman wrote:
>
https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
> File content/renderer/service_worker/service_worker_context_client.cc (right):
> 
>
https://codereview.chromium.org/1318643002/diff/1/content/renderer/service_wo...
> content/renderer/service_worker/service_worker_context_client.cc:401:
> v8::Local<v8::Context> context) {
> On 2015/08/27 00:36:55, kinuko wrote:
> > On 2015/08/26 17:29:02, kalman wrote:
> > > On 2015/08/26 09:04:18, kinuko wrote:
> > > > Looking into https://codereview.chromium.org/1310843006/ when this one's
> > > called
> > > > this v8 context may not be already available.  If this is used only as a
> key
> > > to
> > > > remove the corresponding extension context could we just use a raw
pointer
> > of
> > > > this instance or of proxy_?
> > > 
> > > A raw pointer of a v8::Context? How would that help (and also I don't
think
> v8
> > > likes that).
> > 
> > I was thinking about using a pointer to 'this', ServiceWorkerContextClient,
or
> > whatever that can be used to identify this context.
> 
> I would need to move this into content/public for that to work. v8::Context
> seems to work fine + is consistent.

It could be a void ptr for a key, no? (Just to be clear I'm ok with the current
code as far as it works)

[not at google - send to devlin, 2015-08-27 16:10:54]

Yeah it could be, though v8::Context is more direct and so long as it all works
out (seems to be, I've been testing locally) should be fine.

[not at google - send to devlin, 2015-08-31 17:41:57]

The CQ bit was checked by kalman@chromium.org

[not at google - send to devlin, 2015-08-31 17:41:58]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1318643002/#ps60001
(title: "rebase against blink change")

[commit-bot: I haz the power, 2015-08-31 17:42:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1318643002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1318643002/60001

[commit-bot: I haz the power, 2015-08-31 18:40:48]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-08-31 18:41:26]

Patchset 3 (id:??) landed as
https://crrev.com/4a0bb926cb1d83fab27b6ad866c04ba481f66268
Cr-Commit-Position: refs/heads/master@{#346424}

[kalman_google, 2015-09-02 16:16:38]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/1329703004/ by kalman@google.com.

The reason for reverting is: Appears to be crashing on Canary..

[not at google - send to devlin, 2015-09-02 18:35:58]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/1319603003/ by kalman@chromium.org.

The reason for reverting is: Canary is frequently crashing, likely due to
threading issues with this patch..