Correctly track cross-thread pending FetchEvents.

Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references (via Member<T> and similar.) References
across heaps must instead be kept and handled by explicit
CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=haraken
BUG=702527
Review-Url: https://codereview.chromium.org/2752203005
Cr-Commit-Position: refs/heads/master@{#457970}
Committed: https://chromium.googlesource.com/chromium/src/+/d0ff3d34a5798ddc3db10815d49dc1687e4f866e

[sof, 2017-03-17 21:24:45]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-17 21:25:18]

Dry run: 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-03-17 23:06:06]

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

[commit-bot: I haz the power, 2017-03-17 23:06:07]

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_...)

[sof, 2017-03-18 06:36:14]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-18 06:36:29]

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

[sof, 2017-03-18 07:22:12]

Description was changed from

==========
Correctly track cross-thread pending FetchEvents.

R=
BUG=702527
==========

to

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references. References across heaps must instead be
kept and handled by explicit CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=
BUG=702527
==========

[sof, 2017-03-18 07:24:46]

Description was changed from

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references. References across heaps must instead be
kept and handled by explicit CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=
BUG=702527
==========

to

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references (via Member<T> and similar.) References
across heaps must instead be kept and handled by explicit
CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=
BUG=702527
==========

[sof, 2017-03-18 07:26:03]

sigbjornf@opera.com changed reviewers:
+ falken@chromium.org, horo@chromium.org, oilpan-reviews@chromium.org

[sof, 2017-03-18 07:26:03]

please take a look.

[commit-bot: I haz the power, 2017-03-18 08:11:50]

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

[commit-bot: I haz the power, 2017-03-18 08:11:51]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-03-18 14:42:43]

LGTM

Why didn't we hit the cross-thread-heap-pointer dcheck during the marking phase?
Just by accident?

[sof, 2017-03-18 15:27:54]

On 2017/03/18 14:42:43, haraken wrote:
> LGTM
> 
> Why didn't we hit the cross-thread-heap-pointer dcheck during the marking
phase?
> Just by accident?

I believe so (but don't know), tests do not trigger GCs afterwards for this to
have been detected.

I can't see many layout tests for the feature, but would be interested to know
more about test status
(http/tests/serviceworker/navigation-preload/chromium/use-counter.html seems
relevant, but what else?)

[sof, 2017-03-18 15:47:11]

Description was changed from

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references (via Member<T> and similar.) References
across heaps must instead be kept and handled by explicit
CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=
BUG=702527
==========

to

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references (via Member<T> and similar.) References
across heaps must instead be kept and handled by explicit
CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=haraken
BUG=702527
==========

[sof, 2017-03-18 15:47:16]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2017-03-18 15:47:25]

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-03-18 15:51:56]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489852036360880,
"parent_rev": "3880018ceed2910bb727a9580cd897f5ae4b6844", "commit_rev":
"d0ff3d34a5798ddc3db10815d49dc1687e4f866e"}

[commit-bot: I haz the power, 2017-03-18 15:52:35]

Description was changed from

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references (via Member<T> and similar.) References
across heaps must instead be kept and handled by explicit
CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=haraken
BUG=702527
==========

to

==========
Correctly track cross-thread pending FetchEvents.

ServiceWorkerGlobalScopeProxy is an object that resides on the
main thread heap, but is passed to the embedder and called
on the (service) worker thread. As the object is GC managed
by the main thread, the worker thread cannot update the proxy
object with references to heap objects residing in its heap,
as the per-thread heap design assumes that each heap only keeps
local heap references (via Member<T> and similar.) References
across heaps must instead be kept and handled by explicit
CrossThreadPersistent<> references.

This per-thread heap rule was not being followed for the tracking
of pending FetchEvents; adjust the map representation to do so.

R=haraken
BUG=702527

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

[commit-bot: I haz the power, 2017-03-18 15:52:37]

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

[falken, 2017-03-23 05:42:10]

On 2017/03/18 15:27:54, sof wrote:
> On 2017/03/18 14:42:43, haraken wrote:
> > LGTM
> > 
> > Why didn't we hit the cross-thread-heap-pointer dcheck during the marking
> phase?
> > Just by accident?
> 
> I believe so (but don't know), tests do not trigger GCs afterwards for this to
> have been detected.
> 
> I can't see many layout tests for the feature, but would be interested to know
> more about test status
> (http/tests/serviceworker/navigation-preload/chromium/use-counter.html seems
> relevant, but what else?)

Sorry for lack of response here. Most navigation preload tests are in
external/wpt/service-workers/service-worker/navigation-preload.