Oilpan: add missing leftmost trace()s for GC mixins.

Oilpan: add missing leftmost trace()s for GC mixins.

For classes that are non-abstract (in a Blink sense) and inherit
from GarbageCollectedMixin, add a missing local trace() override.
This so as to insist on there being a trace() in the leftmost vtbl.

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

[sof, 2015-01-06 22:31:01]

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

[sof, 2015-01-06 22:31:02]

Please take a look.

[dcheng, 2015-01-06 23:06:43]

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

[dcheng, 2015-01-06 23:06:44]

https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImpl.h
File Source/web/WebRemoteFrameImpl.h (right):

https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImp...
Source/web/WebRemoteFrameImpl.h:31:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(PlaceholderFrameOwner);
Out of curiosity, why do we need a mixin at all here? FrameOwner has no data
members at all.

[haraken, 2015-01-07 01:44:40]

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

[haraken, 2015-01-07 01:44:41]

LGTM

https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImpl.h
File Source/web/WebRemoteFrameImpl.h (right):

https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImp...
Source/web/WebRemoteFrameImpl.h:31:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(PlaceholderFrameOwner);
On 2015/01/06 23:06:44, dcheng wrote:
> Out of curiosity, why do we need a mixin at all here? FrameOwner has no data
> members at all.

Hmm, you're right. I don't see any reason FrameOwner needs to be a mixin
either... Sigbjorn, can we de-mixin FrameOwner in a follow-up CL?

[haraken, 2015-01-07 01:44:45]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2015-01-07 01:45:30]

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

[commit-bot: I haz the power, 2015-01-07 04:53:51]

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

[sof, 2015-01-07 06:18:01]

https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImpl.h
File Source/web/WebRemoteFrameImpl.h (right):

https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImp...
Source/web/WebRemoteFrameImpl.h:31:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(PlaceholderFrameOwner);
On 2015/01/07 01:44:41, haraken wrote:
> On 2015/01/06 23:06:44, dcheng wrote:
> > Out of curiosity, why do we need a mixin at all here? FrameOwner has no data
> > members at all.
> 
> Hmm, you're right. I don't see any reason FrameOwner needs to be a mixin
> either... Sigbjorn, can we de-mixin FrameOwner in a follow-up CL?

Yes, well caught -- will remove that.

It depends on style to some extent, but something like

class A : public GarbageCollectedMixin { };
class B : public A { public: Member<A> m_a; };
class C : public GarbageCollected<C>, public B {
USING_GARBAGE_COLLECTED_MIXIN(C); ... };

would benefit from using GCMixin as a base for A, irrespective of A having GC
references or not.

[haraken, 2015-01-07 07:22:57]

On 2015/01/07 06:18:01, sof wrote:
>
https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImpl.h
> File Source/web/WebRemoteFrameImpl.h (right):
> 
>
https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImp...
> Source/web/WebRemoteFrameImpl.h:31:
> WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(PlaceholderFrameOwner);
> On 2015/01/07 01:44:41, haraken wrote:
> > On 2015/01/06 23:06:44, dcheng wrote:
> > > Out of curiosity, why do we need a mixin at all here? FrameOwner has no
data
> > > members at all.
> > 
> > Hmm, you're right. I don't see any reason FrameOwner needs to be a mixin
> > either... Sigbjorn, can we de-mixin FrameOwner in a follow-up CL?
> 
> Yes, well caught -- will remove that.
> 
> It depends on style to some extent, but something like
> 
> class A : public GarbageCollectedMixin { };
> class B : public A { public: Member<A> m_a; };
> class C : public GarbageCollected<C>, public B {
> USING_GARBAGE_COLLECTED_MIXIN(C); ... };
> 
> would benefit from using GCMixin as a base for A, irrespective of A having GC
> references or not.

ah, we have Member<FrameOwner>. Then I'd prefer keeping FrameOwner as a GCMixin
too.

[sof, 2015-01-07 07:32:47]

On 2015/01/07 07:22:57, haraken wrote:
> On 2015/01/07 06:18:01, sof wrote:
> >
>
https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImpl.h
> > File Source/web/WebRemoteFrameImpl.h (right):
> > 
> >
>
https://codereview.chromium.org/835363003/diff/1/Source/web/WebRemoteFrameImp...
> > Source/web/WebRemoteFrameImpl.h:31:
> > WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(PlaceholderFrameOwner);
> > On 2015/01/07 01:44:41, haraken wrote:
> > > On 2015/01/06 23:06:44, dcheng wrote:
> > > > Out of curiosity, why do we need a mixin at all here? FrameOwner has no
> data
> > > > members at all.
> > > 
> > > Hmm, you're right. I don't see any reason FrameOwner needs to be a mixin
> > > either... Sigbjorn, can we de-mixin FrameOwner in a follow-up CL?
> > 
> > Yes, well caught -- will remove that.
> > 
> > It depends on style to some extent, but something like
> > 
> > class A : public GarbageCollectedMixin { };
> > class B : public A { public: Member<A> m_a; };
> > class C : public GarbageCollected<C>, public B {
> > USING_GARBAGE_COLLECTED_MIXIN(C); ... };
> > 
> > would benefit from using GCMixin as a base for A, irrespective of A having
GC
> > references or not.
> 
> ah, we have Member<FrameOwner>. Then I'd prefer keeping FrameOwner as a
GCMixin
> too.

Thanks, now I remember why it was added :) Will add a comment next to FrameOwner
to avoid future head scratching.