ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=468240
Committed: https://crrev.com/3d3956c597ea0f675e91e7d9e4ec4695879d3d2e
Cr-Commit-Position: refs/heads/master@{#438967}

[sof, 2016-12-15 13:15:17]

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

[commit-bot: I haz the power, 2016-12-15 13:15:27]

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

[sof, 2016-12-15 13:23:39]

Description was changed from

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks
and is compatible with expected lifetime of 'host objects' in
a detached script environment.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. This
behavior is already provided by the 'standard' Blink wrapper visitors
that v8 invokes during GC, but not with wrapper tracing, which is
what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=
BUG=
==========

to

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
(Doing so is compatible with expected lifetime of 'host objects' in
a detached script environment.)

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=
BUG=
==========

[sof, 2016-12-15 13:31:55]

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

[commit-bot: I haz the power, 2016-12-15 13:32:14]

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

[sof, 2016-12-15 13:43:40]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org, mlippautz@chromium.org

[sof, 2016-12-15 13:43:42]

please take a look.

(Is there some pre-existing issue I can associate this CL with?)

[Michael Lippautz, 2016-12-15 13:57:31]

lgtm from my side

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp
(right):

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp:41: //
returns. This is done to avoid memory leaks. see
nit: Mind copying over the comment? This part of MajorGCWrapperVisitor should go
away after wrapper tracing is enabled.

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:249:
TraceWrappables status=experimental
Not sure what the implications for this one are. We shouldn't run it on the
tests as we still ship with object grouping. Can we hold off on this one?

Having said that, I plan to make the switch to status=stable soon (after getting
https://codereview.chromium.org/2580813002/ in).

[sof, 2016-12-15 14:06:49]

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:249:
TraceWrappables status=experimental
On 2016/12/15 13:57:31, Michael Lippautz wrote:
> Not sure what the implications for this one are. We shouldn't run it on the
> tests as we still ship with object grouping. Can we hold off on this one?
> 
> Having said that, I plan to make the switch to status=stable soon (after
getting
> https://codereview.chromium.org/2580813002/ in).

Getting close, that's great :)

Yes, I'll definitely revert this change, but thought I needed to transition it
from being "status="-less to enable it for layout tests?

[commit-bot: I haz the power, 2016-12-15 14:11:22]

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

[commit-bot: I haz the power, 2016-12-15 14:11:23]

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

[Michael Lippautz, 2016-12-15 14:16:02]

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:249:
TraceWrappables status=experimental
On 2016/12/15 14:06:49, sof wrote:
> On 2016/12/15 13:57:31, Michael Lippautz wrote:
> > Not sure what the implications for this one are. We shouldn't run it on the
> > tests as we still ship with object grouping. Can we hold off on this one?
> > 
> > Having said that, I plan to make the switch to status=stable soon (after
> getting
> > https://codereview.chromium.org/2580813002/ in).
> 
> Getting close, that's great :)
> 
> Yes, I'll definitely revert this change, but thought I needed to transition it
> from being "status="-less to enable it for layout tests?

Yeah, if you want it to test on the trybots. For committing we should turn it
off and watch the fyi bot at
https://build.chromium.org/p/chromium.fyi/builders/WebKit%20Linux%20-%20Trace...
(Ignore the inspector failures)

[sof, 2016-12-15 15:33:02]

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp
(right):

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp:41: //
returns. This is done to avoid memory leaks. see
On 2016/12/15 13:57:31, Michael Lippautz wrote:
> nit: Mind copying over the comment? This part of MajorGCWrapperVisitor should
go
> away after wrapper tracing is enabled.

Blended in that comment. Even though we invoke hasPendingActivity() always, it
is no longer considered a bug if it returns |true| when in a detached state. 

Writing up some general guidelines for hasPendingActivity() implementations,
would be worth doing.

[haraken, 2016-12-15 15:35:06]

LGTM

> Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|.

Just to clarify: This is just a hacky (but very practical) safe guard to prevent
memory leaks. Per the spec, we should keep alive DOM objects as long as the DOM
objects are reachable. The spec is not saying that we can forcibly collect DOM
wrappers when the context is detached.

https://codereview.chromium.org/2577053002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.h (right):

https://codereview.chromium.org/2577053002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.h:54: return
!(static_cast<T*>(object)->T::getExecutionContext)() ||

You're clever!

I'm surprised that all ActiveScriptWrappable objects already have
getExecutionContext)(). Is it because all ActiveScriptWrappable objects inherit
from EventTarget by accident?

[sof, 2016-12-15 15:47:40]

https://codereview.chromium.org/2577053002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.h (right):

https://codereview.chromium.org/2577053002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.h:54: return
!(static_cast<T*>(object)->T::getExecutionContext)() ||
On 2016/12/15 15:35:06, haraken wrote:
> 
> You're clever!
> 
> I'm surprised that all ActiveScriptWrappable objects already have
> getExecutionContext)(). Is it because all ActiveScriptWrappable objects
inherit
> from EventTarget by accident?

Or ContextLifecycleObserver.. or both (in which case it already needs to
disambiguate, so this will behave as wanted.)

[sof, 2016-12-15 19:21:04]

Description was changed from

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
(Doing so is compatible with expected lifetime of 'host objects' in
a detached script environment.)

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=
BUG=
==========

to

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=
==========

[sof, 2016-12-15 19:29:46]

On 2016/12/15 15:35:06, haraken wrote:
> LGTM
> 
> > Consequently, a ScriptWrappable in a detached ExecutionContext should
> not be retained, even if hasPendingActivity() return |true|.
> 
> Just to clarify: This is just a hacky (but very practical) safe guard to
prevent
> memory leaks. Per the spec, we should keep alive DOM objects as long as the
DOM
> objects are reachable. The spec is not saying that we can forcibly collect DOM
> wrappers when the context is detached.
> 

You're right, there isn't a backing for this in the spec. I've adjusted comments
& description to claim less.

[sof, 2016-12-15 19:30:50]

Description was changed from

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=
==========

to

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=468240
==========

[sof, 2016-12-15 19:32:26]

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/2577053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:249:
TraceWrappables status=experimental
On 2016/12/15 14:16:02, Michael Lippautz wrote:
> On 2016/12/15 14:06:49, sof wrote:
> > On 2016/12/15 13:57:31, Michael Lippautz wrote:
> > > Not sure what the implications for this one are. We shouldn't run it on
the
> > > tests as we still ship with object grouping. Can we hold off on this one?
> > > 
> > > Having said that, I plan to make the switch to status=stable soon (after
> > getting
> > > https://codereview.chromium.org/2580813002/ in).
> > 
> > Getting close, that's great :)
> > 
> > Yes, I'll definitely revert this change, but thought I needed to transition
it
> > from being "status="-less to enable it for layout tests?
> 
> Yeah, if you want it to test on the trybots. For committing we should turn it
> off and watch the fyi bot at
>
https://build.chromium.org/p/chromium.fyi/builders/WebKit%20Linux%20-%20Trace...
> (Ignore the inspector failures)

ok, reverted in ps#4.

[sof, 2016-12-15 19:32:39]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-12-15 19:32:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2577053002/#ps60001
(title: "adjust comment + revert TraceWrappables feature change")

[commit-bot: I haz the power, 2016-12-15 19:33:24]

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, 2016-12-15 20:19:51]

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

[commit-bot: I haz the power, 2016-12-15 20:19:54]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[sof, 2016-12-15 22:32:00]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-12-15 22:32:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, mlippautz@chromium.org
Link to the patchset: https://codereview.chromium.org/2577053002/#ps100001
(title: "component build fix(msvc)")

[commit-bot: I haz the power, 2016-12-15 22:33:36]

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, 2016-12-16 00:30:13]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481841120469780,
"parent_rev": "a2e5e97f5ae7ce2858c64b2d88da6894a7a413ef", "commit_rev":
"50912c8833b331907bdea4ae8c02393196019968"}

[commit-bot: I haz the power, 2016-12-16 00:30:41]

Description was changed from

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=468240
==========

to

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=468240

Review-Url: https://codereview.chromium.org/2577053002
==========

[commit-bot: I haz the power, 2016-12-16 00:30:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-16 00:33:12]

Description was changed from

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=468240

Review-Url: https://codereview.chromium.org/2577053002
==========

to

==========
ActiveScriptWrappable: GC wrappers in detached ExecutionContexts.

Blink objects that implement (Active)ScriptWrappable have the ability
to keep their corresponding v8 wrapper object alive across GCs by
overriding and implementing ScriptWrappable::hasPendingActivity().

Once an ExecutionContext has become detached, we no longer want to
retain wrappers belonging to it, as that will lead to memory leaks.
With full bi-directional tracing of references across the v8 and Blink
heaps, it is possible to make the lifetime of objects "more accurate",
but not keeping a wrapper alive once in a detached setting, has proven
to work out well in practice.

Consequently, a ScriptWrappable in a detached ExecutionContext should
not be retained, even if hasPendingActivity() return |true|. That is,
we should simply ignore hasPendingActivity()'s result, freeing the
implementations of it from having to take care of this 'detached'
detail.

This behavior is already provided by the 'standard' Blink wrapper
visitors that v8 invokes during GC, but not with wrapper tracing,
which is what this CL brings.

It does so by extending ActiveScriptWrappable with a predicate for
checking if the object's ExecutionContext has signalled destruction.

(The natural(?) way to express that is to parameterize ActiveScriptWrappable<>
over the class that implements the interface. This makes for a CL
with a larger footprint.)

R=haraken, mlippautz
BUG=468240

Committed: https://crrev.com/3d3956c597ea0f675e91e7d9e4ec4695879d3d2e
Cr-Commit-Position: refs/heads/master@{#438967}
==========

[commit-bot: I haz the power, 2016-12-16 00:33:15]

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