Eliminate superfluous clearing of weak pointer.

Eliminate superfluous clearing of weak pointer.

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

Committed: https://crrev.com/1151b75ea1f6cc0bb5a3937bf830285f68aa25ac
Cr-Commit-Position: refs/heads/master@{#368777}

[dcheng, 2016-01-11 22:29:42]

Note that
    m_weakPointerFactory.clear();
in ElementIntersectionObserverData::dispose() is also superfluous, as dispose()
is only called in the destructor.

https://codereview.chromium.org/1580503003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/IntersectionObservation.cpp (right):

https://codereview.chromium.org/1580503003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/IntersectionObservation.cpp:119: //
Pre-oilpan, there will be a delay between the time when the target Element gets
deleted
It's not accurate to imply that an Element and any IntersectionObservations
targeting it will be deleted in the same GC cycle. Given how complicated this
comment would be though, I'd suggest just leaving it: we can better document how
we avoid calling this on an observation waiting to be swept with the Oilpan
fixes.

[haraken, 2016-01-12 00:01:37]

LGTM

https://codereview.chromium.org/1580503003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/IntersectionObservation.cpp (right):

https://codereview.chromium.org/1580503003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/IntersectionObservation.cpp:123: // hasn't
been cleared.

Nit: This comment looks nice (so I want to keep it) but this is an already
widely-used programming pattern for weak pointers. We need to have a null check
before using a weak pointer unless it is guaranteed that the weak pointer must
not be null in some other way.

[szager1, 2016-01-12 00:12:00]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2016-01-12 00:13:59]

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

[dcheng, 2016-01-12 00:19:56]

My other comment about ElementIntersectionObserverData has not been addressed?

[szager1, 2016-01-12 00:35:08]

The CQ bit was unchecked by szager@chromium.org

[szager1, 2016-01-12 00:35:36]

On 2016/01/12 00:19:56, dcheng wrote:
> My other comment about ElementIntersectionObserverData has not been addressed?

Sorry, I missed that one; removed the call to m_weakPointerFactory.clear().

[szager1, 2016-01-12 00:45:04]

The CQ bit was checked by szager@chromium.org

[szager1, 2016-01-12 00:45:04]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1580503003/#ps40001
(title: "Eliminate superfluous WeakPtrFactory::clear()")

[dcheng, 2016-01-12 00:46:13]

lgtm, thanks

[commit-bot: I haz the power, 2016-01-12 00:46:31]

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

[commit-bot: I haz the power, 2016-01-12 02:44:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-12 02:46:29]

Description was changed from

==========
Eliminate superfluous clearing of weak pointer.

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

to

==========
Eliminate superfluous clearing of weak pointer.

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

Committed: https://crrev.com/1151b75ea1f6cc0bb5a3937bf830285f68aa25ac
Cr-Commit-Position: refs/heads/master@{#368777}
==========

[commit-bot: I haz the power, 2016-01-12 02:46:30]

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