Make ActiveScriptWrappable the GC mixin it is.

Make ActiveScriptWrappable the GC mixin it is.

By switching it to derive from GarbageCollectedMixin, and
have the per-thread set of live wrappables keep WeakMember<>
references, the Oilpan GC takes care of pruning the live
set as part of its weak processing.

R=
BUG=

Committed: https://crrev.com/8bf3416c05e0d27a3151f68cac503de278468da7
Cr-Commit-Position: refs/heads/master@{#392618}

[haraken, 2016-05-10 07:10:26]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-05-10 07:10:27]

LGTM

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp
(right):

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp:20:
DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<ActiveScriptWrappableSetType>,
activeScriptWrappableSet, new ThreadSpecific<ActiveScriptWrappableSetType>());

Not directly related to this CL, shall we move the hash set to V8PerIsolateData?
V8PerIsolateData is to store per-isolate things like this.

[sof, 2016-05-10 15:46:04]

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp
(right):

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ActiveScriptWrappable.cpp:20:
DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<ActiveScriptWrappableSetType>,
activeScriptWrappableSet, new ThreadSpecific<ActiveScriptWrappableSetType>());
On 2016/05/10 07:10:27, haraken wrote:
> 
> Not directly related to this CL, shall we move the hash set to
V8PerIsolateData?
> V8PerIsolateData is to store per-isolate things like this.
> 

That would avoid ThreadSpecific<> , which would be a good thing overall. I can
try to followup.

[sof, 2016-05-10 15:48:17]

Description was changed from

==========
Make ActiveScriptWrappable the GC mixin it is.

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

to

==========
Make ActiveScriptWrappable the GC mixin it is.

By switching it to derive from GarbageCollectedMixin, and
have the per-thread set of live wrappables keep WeakMember<>
references, the Oilpan GC takes care of pruning the live
set as part of its weak processing.

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

[sof, 2016-05-10 15:48:23]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2016-05-10 15:48:34]

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

[commit-bot: I haz the power, 2016-05-10 15:52:17]

Description was changed from

==========
Make ActiveScriptWrappable the GC mixin it is.

By switching it to derive from GarbageCollectedMixin, and
have the per-thread set of live wrappables keep WeakMember<>
references, the Oilpan GC takes care of pruning the live
set as part of its weak processing.

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

to

==========
Make ActiveScriptWrappable the GC mixin it is.

By switching it to derive from GarbageCollectedMixin, and
have the per-thread set of live wrappables keep WeakMember<>
references, the Oilpan GC takes care of pruning the live
set as part of its weak processing.

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

[commit-bot: I haz the power, 2016-05-10 15:52:18]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-10 15:53:20]

Description was changed from

==========
Make ActiveScriptWrappable the GC mixin it is.

By switching it to derive from GarbageCollectedMixin, and
have the per-thread set of live wrappables keep WeakMember<>
references, the Oilpan GC takes care of pruning the live
set as part of its weak processing.

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

to

==========
Make ActiveScriptWrappable the GC mixin it is.

By switching it to derive from GarbageCollectedMixin, and
have the per-thread set of live wrappables keep WeakMember<>
references, the Oilpan GC takes care of pruning the live
set as part of its weak processing.

R=
BUG=

Committed: https://crrev.com/8bf3416c05e0d27a3151f68cac503de278468da7
Cr-Commit-Position: refs/heads/master@{#392618}
==========

[commit-bot: I haz the power, 2016-05-10 15:53:21]

Patchset 1 (id:??) landed as
https://crrev.com/8bf3416c05e0d27a3151f68cac503de278468da7
Cr-Commit-Position: refs/heads/master@{#392618}

[hongchan, 2016-05-10 16:58:28]

hongchan@chromium.org changed reviewers:
+ hongchan@chromium.org

[hongchan, 2016-05-10 16:58:29]

I know the CL is already landed, and I am not the designated reviewer - but I
have one question.

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h (right):

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h:143:
USING_GARBAGE_COLLECTED_MIXIN(AudioBufferSourceNode);
AudioScheduledSourceNode already has mixin. Is this necessary?

If this is necessary, OsciallatorNode also should be taken care of.

[sof, 2016-05-10 19:02:01]

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h (right):

https://codereview.chromium.org/1962113003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h:143:
USING_GARBAGE_COLLECTED_MIXIN(AudioBufferSourceNode);
On 2016/05/10 16:58:28, hoch wrote:
> AudioScheduledSourceNode already has mixin. Is this necessary?
> 
> If this is necessary, OsciallatorNode also should be taken care of.

Thanks for catching that, it is unnecessary as it only extends its only
(leftmost) derived class.

Let's remove it, see https://codereview.chromium.org/1969453003/