Oilpan: Move CSSStyleDeclaration and subclasses to the heap using transistion types.

Oilpan: Move CSSStyleDeclaration and subclasses to the heap using transistion types.

R=ager@chromium.org, erik.corry@gmail.com, haraken@chromium.org, oilpan-reviews@chromium.org, tkent@chromium.org, vegorov@chromium.org, zerny@chromium.org
BUG=341815
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169652

[haraken, 2014-03-20 11:22:38]

LGTM

https://codereview.chromium.org/195953003/diff/1/Source/core/css/CSSStyleDecl...
File Source/core/css/CSSStyleDeclaration.h (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/css/CSSStyleDecl...
Source/core/css/CSSStyleDeclaration.h:73: virtual void trace(Visitor*) { }

Probably this could be a pure virtual function, since all other functions in
CSSStyleDeclaration are pure virtual.

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
File Source/core/css/PropertySetCSSStyleDeclaration.cpp (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.cpp:320: // We don't support
tracing OwnPtr<HashMap<X, Y> > since the hashmaps trace

FIXME: oilpan:

Or you can just drop this comment. We've already added the ENABLE(OILPAN) flag
to a bunch of visitor->trace()'s.

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
File Source/core/css/PropertySetCSSStyleDeclaration.h (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.h:79:
OwnPtrWillBeMember<WillBeHeapHashMap<CSSValue*, RefPtrWillBeMember<CSSValue> > >
m_cssomCSSValueClones;

Shouldn't this be
OwnPtrWillBeMember<WillBeHeapHashMap<RawPtrWillBeMember<CSSValue>,
RefPtrWillBeMember<CSSValue>>> ?

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.h:116: virtual void
trace(Visitor*);

Add OVERRIDE.

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.h:157: Element* m_parentElement;

This raw pointer looks safe since it's cleared in clearParentElement().

https://codereview.chromium.org/195953003/diff/1/Source/core/dom/ElementRareD...
File Source/core/dom/ElementRareData.cpp (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/dom/ElementRareD...
Source/core/dom/ElementRareData.cpp:44:
OwnPtrWillBePersistent<InlineCSSStyleDeclaration> cssomWrapper;

Increasing the size of ElementRareData is sad. We might need to address this
increase when enabling the oilpan flag for the CSS hierarchy.

[wibling-chromium, 2014-03-20 11:59:33]

Thanks for the review~

https://codereview.chromium.org/195953003/diff/1/Source/core/css/CSSStyleDecl...
File Source/core/css/CSSStyleDeclaration.h (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/css/CSSStyleDecl...
Source/core/css/CSSStyleDeclaration.h:73: virtual void trace(Visitor*) { }
On 2014/03/20 11:22:38, haraken wrote:
> 
> Probably this could be a pure virtual function, since all other functions in
> CSSStyleDeclaration are pure virtual.

Done.

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
File Source/core/css/PropertySetCSSStyleDeclaration.cpp (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.cpp:320: // We don't support
tracing OwnPtr<HashMap<X, Y> > since the hashmaps trace
On 2014/03/20 11:22:38, haraken wrote:
> 
> FIXME: oilpan:
> 
> Or you can just drop this comment. We've already added the ENABLE(OILPAN) flag
> to a bunch of visitor->trace()'s.

Removed the comment.

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
File Source/core/css/PropertySetCSSStyleDeclaration.h (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.h:79:
OwnPtrWillBeMember<WillBeHeapHashMap<CSSValue*, RefPtrWillBeMember<CSSValue> > >
m_cssomCSSValueClones;
On 2014/03/20 11:22:38, haraken wrote:
> 
> Shouldn't this be
> OwnPtrWillBeMember<WillBeHeapHashMap<RawPtrWillBeMember<CSSValue>,
> RefPtrWillBeMember<CSSValue>>> ?

Yes, that would be better. Done.

https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
Source/core/css/PropertySetCSSStyleDeclaration.h:116: virtual void
trace(Visitor*);
On 2014/03/20 11:22:38, haraken wrote:
> 
> Add OVERRIDE.

Done.

https://codereview.chromium.org/195953003/diff/1/Source/core/dom/ElementRareD...
File Source/core/dom/ElementRareData.cpp (right):

https://codereview.chromium.org/195953003/diff/1/Source/core/dom/ElementRareD...
Source/core/dom/ElementRareData.cpp:44:
OwnPtrWillBePersistent<InlineCSSStyleDeclaration> cssomWrapper;
On 2014/03/20 11:22:38, haraken wrote:
> 
> Increasing the size of ElementRareData is sad. We might need to address this
> increase when enabling the oilpan flag for the CSS hierarchy.

Yes, thankfully this should be larger while it is a persistent. Once we change
it to OwnPtrWillBeMember the size should be the same even with oilpan enabled.

[wibling-chromium, 2014-03-20 12:06:27]

On 2014/03/20 11:59:33, wibling-chromium wrote:
> Thanks for the review~
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/CSSStyleDecl...
> File Source/core/css/CSSStyleDeclaration.h (right):
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/CSSStyleDecl...
> Source/core/css/CSSStyleDeclaration.h:73: virtual void trace(Visitor*) { }
> On 2014/03/20 11:22:38, haraken wrote:
> > 
> > Probably this could be a pure virtual function, since all other functions in
> > CSSStyleDeclaration are pure virtual.
> 
> Done.
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
> File Source/core/css/PropertySetCSSStyleDeclaration.cpp (right):
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
> Source/core/css/PropertySetCSSStyleDeclaration.cpp:320: // We don't support
> tracing OwnPtr<HashMap<X, Y> > since the hashmaps trace
> On 2014/03/20 11:22:38, haraken wrote:
> > 
> > FIXME: oilpan:
> > 
> > Or you can just drop this comment. We've already added the ENABLE(OILPAN)
flag
> > to a bunch of visitor->trace()'s.
> 
> Removed the comment.
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
> File Source/core/css/PropertySetCSSStyleDeclaration.h (right):
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
> Source/core/css/PropertySetCSSStyleDeclaration.h:79:
> OwnPtrWillBeMember<WillBeHeapHashMap<CSSValue*, RefPtrWillBeMember<CSSValue> >
>
> m_cssomCSSValueClones;
> On 2014/03/20 11:22:38, haraken wrote:
> > 
> > Shouldn't this be
> > OwnPtrWillBeMember<WillBeHeapHashMap<RawPtrWillBeMember<CSSValue>,
> > RefPtrWillBeMember<CSSValue>>> ?
> 
> Yes, that would be better. Done.
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/css/PropertySetC...
> Source/core/css/PropertySetCSSStyleDeclaration.h:116: virtual void
> trace(Visitor*);
> On 2014/03/20 11:22:38, haraken wrote:
> > 
> > Add OVERRIDE.
> 
> Done.
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/dom/ElementRareD...
> File Source/core/dom/ElementRareData.cpp (right):
> 
>
https://codereview.chromium.org/195953003/diff/1/Source/core/dom/ElementRareD...
> Source/core/dom/ElementRareData.cpp:44:
> OwnPtrWillBePersistent<InlineCSSStyleDeclaration> cssomWrapper;
> On 2014/03/20 11:22:38, haraken wrote:
> > 
> > Increasing the size of ElementRareData is sad. We might need to address this
> > increase when enabling the oilpan flag for the CSS hierarchy.
> 
> Yes, thankfully this should be larger while it is a persistent. Once we change
> it to OwnPtrWillBeMember the size should be the same even with oilpan enabled.

"should be larger" -> "should only be larger"

[Mads Ager (chromium), 2014-03-20 12:06:31]

LGTM

[wibling-chromium, 2014-03-20 12:23:25]

The CQ bit was checked by wibling@chromium.org

[commit-bot: I haz the power, 2014-03-20 12:23:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wibling@chromium.org/195953003/20001

[commit-bot: I haz the power, 2014-03-20 13:28:51]

Change committed as 169652

[wibling-chromium, 2014-03-20 15:52:42]

A revert of this CL has been created in
https://codereview.chromium.org/205033007/ by wibling@chromium.org.

The reason for reverting is: Breaking tests..