Oilpan: move Console objects to the oilpan heap.

Oilpan: move Console objects to the oilpan heap.

Turn the ConsoleBase object into a garbage collected object if Oilpan
is enabled, avoiding its ref forwarding to either Console or WorkerConsole.

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

[sof, 2014-03-20 14:09:21]

Please take a look.

Disabling the ref-forwarding machinery isn't pretty, but I don't think we want
to leave that in when Oilpan is enabled.

[haraken, 2014-03-20 14:13:06]

LGTM

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBase.h
File Source/core/frame/ConsoleBase.h (right):

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBas...
Source/core/frame/ConsoleBase.h:83: #endif

I'd just remove protected: and make the destructor public even in the non-oilpan
build.

[Mads Ager (chromium), 2014-03-20 14:20:00]

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBase.h
File Source/core/frame/ConsoleBase.h (right):

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBas...
Source/core/frame/ConsoleBase.h:51: void ref() { refConsole(); }
WAT!?! What is this, I don't even?!?

Why isn't ConsoleBase just RefCounted<ConsoleBase>? I would prefer if we can
clean this code up both for Oilpan and without Oilpan. However, I might be
missing something?

https://codereview.chromium.org/206093005/diff/1/Source/core/workers/WorkerCo...
File Source/core/workers/WorkerConsole.h (right):

https://codereview.chromium.org/206093005/diff/1/Source/core/workers/WorkerCo...
Source/core/workers/WorkerConsole.h:49: public RefCounted<WorkerConsole>,
You used a 4-space indent in the other files. Consistently do that?

[sof, 2014-03-20 14:57:23]

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBase.h
File Source/core/frame/ConsoleBase.h (right):

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBas...
Source/core/frame/ConsoleBase.h:51: void ref() { refConsole(); }
On 2014/03/20 14:20:01, Mads Ager (chromium) wrote:
> WAT!?! What is this, I don't even?!?
> 
> Why isn't ConsoleBase just RefCounted<ConsoleBase>? I would prefer if we can
> clean this code up both for Oilpan and without Oilpan. However, I might be
> missing something?

Not missing anything, it definitely can be simplified so. Seems like extra
generality that turned out to be unnecessary when adding support for Console to
Workers.

Very well observed & questioned :) Made ConsoleBase derive from the transition
type RefCountedWillBeGarbageCollectedFinalized instead.

https://codereview.chromium.org/206093005/diff/1/Source/core/frame/ConsoleBas...
Source/core/frame/ConsoleBase.h:83: #endif
On 2014/03/20 14:13:06, haraken wrote:
> 
> I'd just remove protected: and make the destructor public even in the
non-oilpan
> build.

yes; with RefCounted<> now on ConsoleBase the slightest concern I had about
doing this is gone.

[Mads Ager (chromium), 2014-03-20 15:00:30]

LGTM!!!

https://codereview.chromium.org/206093005/diff/40001/Source/core/frame/Consol...
File Source/core/frame/ConsoleBase.h (right):

https://codereview.chromium.org/206093005/diff/40001/Source/core/frame/Consol...
Source/core/frame/ConsoleBase.h:74: virtual void trace(Visitor*) { }
You could make this a pure virtual since there are pure virtual methods already.
Then you don't have to call it in subclasses.

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

https://codereview.chromium.org/206093005/diff/40001/Source/core/frame/Consol...
File Source/core/frame/ConsoleBase.h (right):

https://codereview.chromium.org/206093005/diff/40001/Source/core/frame/Consol...
Source/core/frame/ConsoleBase.h:74: virtual void trace(Visitor*) { }
On 2014/03/20 15:00:31, Mads Ager (chromium) wrote:
> You could make this a pure virtual since there are pure virtual methods
already.
> Then you don't have to call it in subclasses.

Yes, that simplifies a bit further.

[sof, 2014-03-20 15:50:46]

haraken: happy with the resulting simplifications? :)

[haraken, 2014-03-20 16:39:13]

On 2014/03/20 15:50:46, sof wrote:
> haraken: happy with the resulting simplifications? :)

Yes, looking much better. LGTM.

[sof, 2014-03-20 16:41:26]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-03-20 16:41:28]

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

[commit-bot: I haz the power, 2014-03-20 17:48:08]

Change committed as 169670