Oilpan: move Screen and its supplement to the oilpan heap.

Oilpan: move Screen and its supplement to the oilpan heap.

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

[sof, 2014-03-14 11:27:42]

Please take a look.

(Nibbling at core/frame/ )

[zerny-chromium, 2014-03-14 11:31:23]

lgtm

[haraken, 2014-03-14 11:39:45]

LGTM

https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp
File Source/core/frame/Screen.cpp (right):

https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp...
Source/core/frame/Screen.cpp:137: #if ENABLE(OILPAN)

Is this flag needed?

https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
File Source/modules/screen_orientation/ScreenOrientation.h (right):

https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
Source/modules/screen_orientation/ScreenOrientation.h:36: virtual void
trace(Visitor*) { }

Don't you need to trace WillBeHeapSupplement::trace() ?

[zerny-chromium, 2014-03-14 11:54:48]

https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp
File Source/core/frame/Screen.cpp (right):

https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp...
Source/core/frame/Screen.cpp:137: #if ENABLE(OILPAN)
On 2014/03/14 11:39:45, haraken wrote:
> 
> Is this flag needed?

This is needed after a recent change because Supplementable no longer has a
trace method for the non GC case.

https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
File Source/modules/screen_orientation/ScreenOrientation.h (right):

https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
Source/modules/screen_orientation/ScreenOrientation.h:36: virtual void
trace(Visitor*) { }
On 2014/03/14 11:39:45, haraken wrote:
> 
> Don't you need to trace WillBeHeapSupplement::trace() ?

Yes. I missed that one. The ToT plugin should catch this one (trying out a local
build now)

[sof, 2014-03-14 12:01:30]

https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp
File Source/core/frame/Screen.cpp (right):

https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp...
Source/core/frame/Screen.cpp:137: #if ENABLE(OILPAN)
On 2014/03/14 11:39:45, haraken wrote:
> 
> Is this flag needed?

Yes, we don't add a virtual trace() for the non-heap Supplementables.

https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
File Source/modules/screen_orientation/ScreenOrientation.h (right):

https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
Source/modules/screen_orientation/ScreenOrientation.h:36: virtual void
trace(Visitor*) { }
On 2014/03/14 11:54:48, zerny-chromium wrote:
> On 2014/03/14 11:39:45, haraken wrote:
> > 
> > Don't you need to trace WillBeHeapSupplement::trace() ?
> 
> Yes. I missed that one. The ToT plugin should catch this one (trying out a
local
> build now)

Why? Isn't that the method that's implemented here?

[zerny-chromium, 2014-03-14 12:02:51]

On 2014/03/14 12:01:30, sof wrote:
> https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp
> File Source/core/frame/Screen.cpp (right):
> 
>
https://codereview.chromium.org/196023016/diff/1/Source/core/frame/Screen.cpp...
> Source/core/frame/Screen.cpp:137: #if ENABLE(OILPAN)
> On 2014/03/14 11:39:45, haraken wrote:
> > 
> > Is this flag needed?
> 
> Yes, we don't add a virtual trace() for the non-heap Supplementables.
> 
>
https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
> File Source/modules/screen_orientation/ScreenOrientation.h (right):
> 
>
https://codereview.chromium.org/196023016/diff/1/Source/modules/screen_orient...
> Source/modules/screen_orientation/ScreenOrientation.h:36: virtual void
> trace(Visitor*) { }
> On 2014/03/14 11:54:48, zerny-chromium wrote:
> > On 2014/03/14 11:39:45, haraken wrote:
> > > 
> > > Don't you need to trace WillBeHeapSupplement::trace() ?
> > 
> > Yes. I missed that one. The ToT plugin should catch this one (trying out a
> local
> > build now)
> 
> Why? Isn't that the method that's implemented here?

Oh, yes. It is pure-virtual in HeapSupplement. Sorry for the noise.

[sof, 2014-03-14 12:04:42]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-03-14 12:04:49]

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

[commit-bot: I haz the power, 2014-03-14 13:08:29]

Change committed as 169246