Oilpan: prefer eager finalization over prefinalizers.

Oilpan: prefer eager finalization over prefinalizers.

Eagerly finalized objects have the ability to access other
heap objects in their destructors, provided they're not
other eagerly finalized objects. With that partial
ordering on finalization in place, we can recast our current
uses of "prefinalizer" actions to use eager finalization
instead.

Performing that switch simplifies, avoiding the overhead of
prefinalizers of having to separately provide a finalization
method (separate from the destructor.)

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

[sof, 2015-06-03 21:51:49]

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

[sof, 2015-06-03 21:51:49]

+oilpan-reviews: please take a look.

[haraken, 2015-06-04 01:02:30]

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

[haraken, 2015-06-04 01:02:31]

LGTM

https://codereview.chromium.org/1148383012/diff/1/Source/bindings/core/v8/Scr...
File Source/bindings/core/v8/ScriptPromiseResolver.h (right):

https://codereview.chromium.org/1148383012/diff/1/Source/bindings/core/v8/Scr...
Source/bindings/core/v8/ScriptPromiseResolver.h:47: assertNotPending();

Now you can inline assertNotPending() here.

https://codereview.chromium.org/1148383012/diff/1/Source/core/dom/MutationObs...
File Source/core/dom/MutationObserver.h (right):

https://codereview.chromium.org/1148383012/diff/1/Source/core/dom/MutationObs...
Source/core/dom/MutationObserver.h:98: EAGERLY_FINALIZE();

Add a comment on why we want to eagerly finalize?

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:432: // Oilpan: should the LocalDOMWindow
be GCed along with its LocalFrame without the

I'm not sure if it makes any difference or not, but we might want to move this
code at the head of the destructor, to keep the order in which we had been
executing these things.

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.h (right):

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.h:76: EAGERLY_FINALIZE();

Add a comment. Ditto for the other EAGERLY_FINALIZE().

https://codereview.chromium.org/1148383012/diff/1/Source/platform/speech/Plat...
File Source/platform/speech/PlatformSpeechSynthesizer.cpp (left):

https://codereview.chromium.org/1148383012/diff/1/Source/platform/speech/Plat...
Source/platform/speech/PlatformSpeechSynthesizer.cpp:63: //
m_sweepSpeechSynthesizer with a dangling pointer to a freed object.

I'd keep this comment on EAGERLY_FINALIZE(). Having this kind of comment is
helpful to understand why we need the macro.

[sof, 2015-06-04 07:54:39]

https://codereview.chromium.org/1148383012/diff/1/Source/core/dom/MutationObs...
File Source/core/dom/MutationObserver.h (right):

https://codereview.chromium.org/1148383012/diff/1/Source/core/dom/MutationObs...
Source/core/dom/MutationObserver.h:98: EAGERLY_FINALIZE();
On 2015/06/04 01:02:31, haraken wrote:
> 
> Add a comment on why we want to eagerly finalize?

Added. (I couldn't come up with anything meaningful to say without recounting
the destructor code, which is why I initially left it out.)

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.cpp (left):

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:432: // Oilpan: should the LocalDOMWindow
be GCed along with its LocalFrame without the
On 2015/06/04 01:02:31, haraken wrote:
> 
> I'm not sure if it makes any difference or not, but we might want to move this
> code at the head of the destructor, to keep the order in which we had been
> executing these things.
> 
> 

Yes, we really don't need this non-Oilpan; moved it up.

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.h (right):

https://codereview.chromium.org/1148383012/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.h:76: EAGERLY_FINALIZE();
On 2015/06/04 01:02:31, haraken wrote:
> 
> Add a comment. Ditto for the other EAGERLY_FINALIZE().

Done.

[sof, 2015-06-04 07:54:43]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-06-04 07:54:44]

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/1148383012/#ps20001
(title: "expand&improve comments")

[commit-bot: I haz the power, 2015-06-04 07:55:25]

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

[commit-bot: I haz the power, 2015-06-04 08:53:46]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196483