Create retainment infos from handles, even for tracing

Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240, chromium:644213
Committed: https://crrev.com/c5b5bae67033e352f8ee94c550bc326cc87a7147
Cr-Commit-Position: refs/heads/master@{#422790}

[Michael Lippautz, 2016-10-04 10:38:53]

Description was changed from

==========
Create retainment infos from handles, even for tracing

BUG=
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=
==========

[Michael Lippautz, 2016-10-04 10:39:54]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240
==========

[Michael Lippautz, 2016-10-04 10:41:15]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

Long term, we should only keep m_constructRetainedObjectInfos path if we want to
preserve heap profiler behavior.

BUG=chromium:468240
==========

[Michael Lippautz, 2016-10-04 10:41:32]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

Long term, we should only keep m_constructRetainedObjectInfos path if we want to
preserve heap profiler behavior.

BUG=chromium:468240
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

Long term we should only keep m_constructRetainedObjectInfos path if we want to
preserve heap profiler behavior.

BUG=chromium:468240
==========

[Michael Lippautz, 2016-10-04 10:41:51]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

Long term we should only keep m_constructRetainedObjectInfos path if we want to
preserve heap profiler behavior.

BUG=chromium:468240
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

Long term we should only keep m_constructRetainedObjectInfos path if we want to
preserve heap profiler behavior.

BUG=chromium:468240
==========

[Michael Lippautz, 2016-10-04 10:41:52]

mlippautz@chromium.org changed reviewers:
+ haraken@chromium.org, hlopko@chromium.org, hpayer@chromium.org

[Michael Lippautz, 2016-10-04 10:42:51]

ptal

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

https://codereview.chromium.org/2387373002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp:201: if
(node->hasEventListeners())
This branch should also be guarded with m_constructImplicitReferenceGroups.

Then the part of the test at
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/inspector...
would need to be ripped out since we are not retained by HTMLBodyElement
anymore.

wdyt?

(If we decide to also guard it and change the test I can rewrite this branches
in a nicer way.)

[Michael Lippautz, 2016-10-04 11:02:36]

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

https://codereview.chromium.org/2387373002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp:201: if
(node->hasEventListeners())
On 2016/10/04 10:42:51, Michael Lippautz wrote:
> This branch should also be guarded with m_constructImplicitReferenceGroups.
> 
> Then the part of the test at
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/inspector...
> would need to be ripped out since we are not retained by HTMLBodyElement
> anymore.
> 
> wdyt?
> 
> (If we decide to also guard it and change the test I can rewrite this branches
> in a nicer way.)

Sorry for the confusion but if we want to stay compatible this should not be
guarded (althouh it creates an implicit ref group) as we also create retainer
infos for it (line 207+208).

[haraken, 2016-10-04 11:24:48]

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

https://codereview.chromium.org/2387373002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp:210: if
(m_constructImplicitReferenceGroups) {

I'd directly use !RuntimeEnabledFeatures::traceWrappablesEnabled() and avoid
introducing an indirection variable like m_constructImplicitReferenceGroups.

https://codereview.chromium.org/2387373002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp:281: bool
constructImplicitReferenceGroups) {

constructImplicitReferenceGroups is unused.

[Michael Lippautz, 2016-10-04 11:31:03]

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

https://codereview.chromium.org/2387373002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp:210: if
(m_constructImplicitReferenceGroups) {
On 2016/10/04 11:24:48, haraken wrote:
> 
> I'd directly use !RuntimeEnabledFeatures::traceWrappablesEnabled() and avoid
> introducing an indirection variable like m_constructImplicitReferenceGroups.

Done.

https://codereview.chromium.org/2387373002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp:281: bool
constructImplicitReferenceGroups) {
On 2016/10/04 11:24:48, haraken wrote:
> 
> constructImplicitReferenceGroups is unused.

Done.

[haraken, 2016-10-04 11:37:46]

LGTM

[Michael Lippautz, 2016-10-04 13:18:01]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

Long term we should only keep m_constructRetainedObjectInfos path if we want to
preserve heap profiler behavior.

BUG=chromium:468240
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240
==========

[Michael Lippautz, 2016-10-04 13:18:25]

fyi: jochen

[Michael Lippautz, 2016-10-04 13:18:29]

The CQ bit was checked by mlippautz@chromium.org

[commit-bot: I haz the power, 2016-10-04 13:18:52]

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

[Michael Lippautz, 2016-10-04 13:21:09]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240,chromium:644213
==========

[commit-bot: I haz the power, 2016-10-04 14:42:19]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240,chromium:644213
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240,chromium:644213
==========

[commit-bot: I haz the power, 2016-10-04 14:42:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-04 14:44:40]

Description was changed from

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240,chromium:644213
==========

to

==========
Create retainment infos from handles, even for tracing

For object tracing during heap profiling we still need to create retainment
infos to preserve current behavior.

BUG=chromium:468240,chromium:644213

Committed: https://crrev.com/c5b5bae67033e352f8ee94c550bc326cc87a7147
Cr-Commit-Position: refs/heads/master@{#422790}
==========

[commit-bot: I haz the power, 2016-10-04 14:44:41]

Patchset 2 (id:??) landed as
https://crrev.com/c5b5bae67033e352f8ee94c550bc326cc87a7147
Cr-Commit-Position: refs/heads/master@{#422790}