Oilpan: move TextFinder to the heap.

Oilpan: move TextFinder to the heap.

With WebLocalFrameImpl on the heap, this object no longer needs to use
persistent (or untraced) references.

R=haraken
BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184332

[sof, 2014-10-23 17:16:09]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2014-10-23 17:16:09]

Please take a look.

[haraken, 2014-10-24 01:18:20]

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

[haraken, 2014-10-24 01:18:21]

LGTM

https://codereview.chromium.org/674773003/diff/1/Source/web/TextFinder.cpp
File Source/web/TextFinder.cpp (right):

https://codereview.chromium.org/674773003/diff/1/Source/web/TextFinder.cpp#ne...
Source/web/TextFinder.cpp:80: void dispose()

Actually this dispose() method wouldn't be needed. There is no issue in
(implicitly) deleting m_timer in DeferredScopeStringMatches's destructor.

https://codereview.chromium.org/674773003/diff/1/Source/web/WebLocalFrameImpl...
File Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/674773003/diff/1/Source/web/WebLocalFrameImpl...
Source/web/WebLocalFrameImpl.cpp:1541: cancelPendingScopingEffort();

Just to confirm: It is ok not to call cancelPendingScopingEffort() (which
changes TextFinder::m_activeMatchIndexInCurrentFrame etc) because the
WebLocalFrameImpl and TextFinder die together, right?

[sof, 2014-10-24 05:32:00]

https://codereview.chromium.org/674773003/diff/1/Source/web/TextFinder.cpp
File Source/web/TextFinder.cpp (right):

https://codereview.chromium.org/674773003/diff/1/Source/web/TextFinder.cpp#ne...
Source/web/TextFinder.cpp:80: void dispose()
On 2014/10/24 01:18:21, haraken wrote:
> 
> Actually this dispose() method wouldn't be needed. There is no issue in
> (implicitly) deleting m_timer in DeferredScopeStringMatches's destructor.

Yes, that will happen as expectefd. And is now done, cf. the destructor no
longer doing an explicit cancel.

The need for dispose() is to handle cancellations that happen in other settings;
they need to pre-emptively dispose/cancel the timers on these deferred
operations.

https://codereview.chromium.org/674773003/diff/1/Source/web/WebLocalFrameImpl...
File Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/674773003/diff/1/Source/web/WebLocalFrameImpl...
Source/web/WebLocalFrameImpl.cpp:1541: cancelPendingScopingEffort();
On 2014/10/24 01:18:21, haraken wrote:
> 
> Just to confirm: It is ok not to call cancelPendingScopingEffort() (which
> changes TextFinder::m_activeMatchIndexInCurrentFrame etc) because the
> WebLocalFrameImpl and TextFinder die together, right?

Correct - the TextFinder is owned&controlled by WebLocalFrameImpl; they die
together.

[sof, 2014-10-24 06:22:33]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-10-24 06:23:14]

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

[commit-bot: I haz the power, 2014-10-24 07:03:19]

Committed patchset #1 (id:1) as 184332