Oilpan: make DocumentFragment a heap allocated object.

Oilpan: make DocumentFragment a heap allocated object.

This relands r174382, as it was reverted while investigating a test failure.

R=wibling@chromium.org,zerny@chromium.org,haraken@chromium.org
BUG=357163
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174443

[sof, 2014-05-18 21:18:15]

Please take a look.

This is based on keishi's two CLs ( https://codereview.chromium.org/288443006/ +
https://codereview.chromium.org/283663006/), but without turning
TemplateContentDocumentFragment::create()'s Element argument into a reference.
Will rebase once CQ lets them through.

[haraken, 2014-05-18 22:07:35]

I'd like to take another look after keishi@'s CLs land.

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
File Source/core/dom/DocumentFragment.idl (right):

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
Source/core/dom/DocumentFragment.idl:23: WillBeGarbageCollected,

What happens if you make Node [WillBeGarbageCollected]?

There are a lot of overloaded methods (e.g., toV8, wrap etc) and I'm not sure if
it's safe to make only a part of the Node hierarchy [WillBeGarbageCollected]. To
avoid the confusion, we might want to make Node [WillBeGarbageCollected] first,
instead of making some sub-Node [WillBeGarbageCollected].

[sof, 2014-05-20 09:19:55]

The two CLs has now landed; rebased.

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
File Source/core/dom/DocumentFragment.idl (right):

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
Source/core/dom/DocumentFragment.idl:23: WillBeGarbageCollected,
On 2014/05/18 22:07:36, haraken wrote:
> 
> What happens if you make Node [WillBeGarbageCollected]?
> 
> There are a lot of overloaded methods (e.g., toV8, wrap etc) and I'm not sure
if
> it's safe to make only a part of the Node hierarchy [WillBeGarbageCollected].
To
> avoid the confusion, we might want to make Node [WillBeGarbageCollected]
first,
> instead of making some sub-Node [WillBeGarbageCollected].

That will trigger some amount of work, moving various RefPtrs over Nodes and
Elements over. I've not taken it on here, but could do that separately, before
or after?

[sof, 2014-05-20 09:22:08]

On 2014/05/20 09:19:55, sof wrote:
> The two CLs has now landed; rebased.
> 

sorry, s/has/have - me write proper one day. :)

[haraken, 2014-05-20 09:32:32]

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
File Source/core/dom/DocumentFragment.idl (right):

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
Source/core/dom/DocumentFragment.idl:23: WillBeGarbageCollected,
On 2014/05/20 09:19:55, sof wrote:
> On 2014/05/18 22:07:36, haraken wrote:
> > 
> > What happens if you make Node [WillBeGarbageCollected]?
> > 
> > There are a lot of overloaded methods (e.g., toV8, wrap etc) and I'm not
sure
> if
> > it's safe to make only a part of the Node hierarchy
[WillBeGarbageCollected].
> To
> > avoid the confusion, we might want to make Node [WillBeGarbageCollected]
> first,
> > instead of making some sub-Node [WillBeGarbageCollected].
> 
> That will trigger some amount of work, moving various RefPtrs over Nodes and
> Elements over. I've not taken it on here, but could do that separately, before
> or after?

I checked the binding code and confirmed that adding [WillBeGarbageCollected] to
a part of the hierarchy won't confuse overload resolution. (We're already doing
that for DOM objects that inherit from EventTarget.) So after is OK, but the
eariler, the better :)

Shall we add a FIXME and mention that the [WillBeGarbageCollected] should be
removed.

[haraken, 2014-05-20 09:33:18]

(will take a close look in hours.)

[wibling-chromium, 2014-05-20 10:31:19]

lgtm

[zerny-chromium, 2014-05-20 10:58:26]

lgtm

[haraken, 2014-05-20 12:53:06]

LGTM

[sof, 2014-05-20 13:15:03]

Thanks for the reviews.

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
File Source/core/dom/DocumentFragment.idl (right):

https://codereview.chromium.org/289273002/diff/20001/Source/core/dom/Document...
Source/core/dom/DocumentFragment.idl:23: WillBeGarbageCollected,
On 2014/05/20 09:32:32, haraken wrote:
> On 2014/05/20 09:19:55, sof wrote:
> > On 2014/05/18 22:07:36, haraken wrote:
> > > 
> > > What happens if you make Node [WillBeGarbageCollected]?
> > > 
> > > There are a lot of overloaded methods (e.g., toV8, wrap etc) and I'm not
> sure
> > if
> > > it's safe to make only a part of the Node hierarchy
> [WillBeGarbageCollected].
> > To
> > > avoid the confusion, we might want to make Node [WillBeGarbageCollected]
> > first,
> > > instead of making some sub-Node [WillBeGarbageCollected].
> > 
> > That will trigger some amount of work, moving various RefPtrs over Nodes and
> > Elements over. I've not taken it on here, but could do that separately,
before
> > or after?
> 
> I checked the binding code and confirmed that adding [WillBeGarbageCollected]
to
> a part of the hierarchy won't confuse overload resolution. (We're already
doing
> that for DOM objects that inherit from EventTarget.) So after is OK, but the
> eariler, the better :)
> 
> Shall we add a FIXME and mention that the [WillBeGarbageCollected] should be
> removed.

ok, thanks - added the FIXME.

[sof, 2014-05-20 13:15:11]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-05-20 13:15:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/289273002/60001

[commit-bot: I haz the power, 2014-05-20 14:45:03]

Change committed as 174382

[sof, 2014-05-21 06:16:30]

Reopened to reland, the revert in https://codereview.chromium.org/291233003/
turned out not to be behind the test failure, see
http://code.google.com/p/chromium/issues/detail?id=375489.

[sof, 2014-05-21 06:18:42]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-05-21 06:19:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/289273002/60001

[commit-bot: I haz the power, 2014-05-21 06:19:51]

Change committed as 174443