Use registerWeakMembers to clean up IntersectionObserver.

Use registerWeakMembers to clean up IntersectionObserver.

Pre-oilpan, when the root disappears, it observers are cleaned up
in NodeIntersectionObserverData::dispose().

Post-oilpan, the cleanup will be done with registerWeakMembers.

BUG=540528
R=haraken@chromium.org,dcheng@chromium.org

Committed: https://crrev.com/546573354e47ddd01f491c382701fa51500f85eb
Cr-Commit-Position: refs/heads/master@{#369948}

[haraken, 2016-01-16 04:11:09]

LGTM

[szager1, 2016-01-16 06:31:56]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2016-01-16 06:32:38]

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

[commit-bot: I haz the power, 2016-01-16 07:36:32]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-16 07:37:19]

Description was changed from

==========
Use registerWeakMembers to clean up IntersectionObserver.

Pre-oilpan, when the root disappears, it observers are cleaned up
in NodeIntersectionObserverData::dispose().

Post-oilpan, the cleanup will be done with registerWeakMembers.

BUG=540528
R=haraken@chromium.org,dcheng@chromium.org
==========

to

==========
Use registerWeakMembers to clean up IntersectionObserver.

Pre-oilpan, when the root disappears, it observers are cleaned up
in NodeIntersectionObserverData::dispose().

Post-oilpan, the cleanup will be done with registerWeakMembers.

BUG=540528
R=haraken@chromium.org,dcheng@chromium.org

Committed: https://crrev.com/546573354e47ddd01f491c382701fa51500f85eb
Cr-Commit-Position: refs/heads/master@{#369948}
==========

[commit-bot: I haz the power, 2016-01-16 07:37:20]

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

[sof, 2016-01-16 09:17:18]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2016-01-16 09:17:18]

https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserver.cpp (right):

https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserver.cpp:328: #if
ENABLE(OILPAN)
How is this supposed to work? A weak callback is registered for m_root and we
trace it separately. Doesn't that mean the m_root could be cleared before the
weak callback is invoked?

[haraken, 2016-01-16 09:25:18]

https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserver.cpp (right):

https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserver.cpp:332:
visitor->trace(m_root);

Good point... We need to remove this.

[sof, 2016-01-16 09:26:59]

On 2016/01/16 09:25:18, haraken wrote:
>
https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/dom/IntersectionObserver.cpp (right):
> 
>
https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/dom/IntersectionObserver.cpp:332:
> visitor->trace(m_root);
> 
> Good point... We need to remove this.

Not just that, but check if the object is alive in the weak callback.

This CL doesn't make much sense, yet it was approved.

[haraken, 2016-01-16 11:24:17]

On 2016/01/16 09:26:59, sof wrote:
> On 2016/01/16 09:25:18, haraken wrote:
> >
>
https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/dom/IntersectionObserver.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1591763003/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/dom/IntersectionObserver.cpp:332:
> > visitor->trace(m_root);
> > 
> > Good point... We need to remove this.
> 
> Not just that, but check if the object is alive in the weak callback.
> 
> This CL doesn't make much sense, yet it was approved.

Yeah, I did a poor review... Let me fix it in a couple hours.

[sof, 2016-01-17 21:39:46]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1600243002/ by sigbjornf@opera.com.

The reason for reverting is: Broke on some tests w/ Oilpan enabled,


https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Oilpan...

backing it out.

(see also https://codereview.chromium.org/1596333002/ ).