Oilpan: fix build after r183816

Oilpan: fix build after r183816

R=haraken
BUG=
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183825

[sof, 2014-10-16 15:50:22]

Please take a look.

The problem with these "const auto&" range loops is due to having
HashTraits<Member<T>>::IteratorConstReferenceType map to T* with Oilpan, which
leads to confusion when iterating a type like

   WillBeHeapHashSet<RefPtrWillBeMember<Widget>>

as the same hash trait type for RefPtr<> keeps the const reference as a
RefPtr<>.

I assume the Oilpan mapping of T* is that way for a good reason, and am leaving
it alone here.

[haraken, 2014-10-16 16:02:23]

LGTM

[sof, 2014-10-16 16:06:44]

On 2014/10/16 16:02:23, haraken wrote:
> LGTM

many thanks; too many breakages atm.

[sof, 2014-10-16 16:06:52]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-10-16 16:07:39]

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

[haraken, 2014-10-16 16:07:50]

> many thanks; too many breakages atm.

Thanks for fixing! I'm heading for bed, so feel free to land trivial fixes with
TBR.

[commit-bot: I haz the power, 2014-10-16 16:08:21]

Committed patchset #2 (id:20001) as 183825

[sof, 2014-10-16 16:12:07]

On 2014/10/16 16:07:50, haraken wrote:
> > many thanks; too many breakages atm.
> 
> Thanks for fixing! I'm heading for bed, so feel free to land trivial fixes
with
> TBR.

Will do, thanks.

[haraken, 2014-12-16 07:57:44]

On 2014/10/16 15:50:22, sof wrote:
> Please take a look.
> 
> The problem with these "const auto&" range loops is due to having
> HashTraits<Member<T>>::IteratorConstReferenceType map to T* with Oilpan, which
> leads to confusion when iterating a type like
> 
>    WillBeHeapHashSet<RefPtrWillBeMember<Widget>>
> 
> as the same hash trait type for RefPtr<> keeps the const reference as a
> RefPtr<>.
> 
> I assume the Oilpan mapping of T* is that way for a good reason, and am
leaving
> it alone here.

Hmm. Actually I'm not quite sure of the "good reason". Do you have any idea on
it? I mean, I want to understand how tasak's fix
(https://codereview.chromium.org/808673005/) affects performance.

[sof, 2014-12-16 08:06:23]

On 2014/12/16 07:57:44, haraken wrote:
> On 2014/10/16 15:50:22, sof wrote:
> > Please take a look.
> > 
> > The problem with these "const auto&" range loops is due to having
> > HashTraits<Member<T>>::IteratorConstReferenceType map to T* with Oilpan,
which
> > leads to confusion when iterating a type like
> > 
> >    WillBeHeapHashSet<RefPtrWillBeMember<Widget>>
> > 
> > as the same hash trait type for RefPtr<> keeps the const reference as a
> > RefPtr<>.
> > 
> > I assume the Oilpan mapping of T* is that way for a good reason, and am
> leaving
> > it alone here.
> 
> Hmm. Actually I'm not quite sure of the "good reason". Do you have any idea on
> it? I mean, I want to understand how tasak's fix
> (https://codereview.chromium.org/808673005/) affects performance.

I want you and tasak to assure me that there isn't anything to be concerned
with. I pointed out the mismatch here & left alone what seemed like an
intentional const-ref mapping for this trait, as it seemed to be that way on
purpose comparing to the other mappings surrounding it.

[haraken, 2014-12-16 08:08:53]

On 2014/12/16 08:06:23, sof wrote:
> On 2014/12/16 07:57:44, haraken wrote:
> > On 2014/10/16 15:50:22, sof wrote:
> > > Please take a look.
> > > 
> > > The problem with these "const auto&" range loops is due to having
> > > HashTraits<Member<T>>::IteratorConstReferenceType map to T* with Oilpan,
> which
> > > leads to confusion when iterating a type like
> > > 
> > >    WillBeHeapHashSet<RefPtrWillBeMember<Widget>>
> > > 
> > > as the same hash trait type for RefPtr<> keeps the const reference as a
> > > RefPtr<>.
> > > 
> > > I assume the Oilpan mapping of T* is that way for a good reason, and am
> > leaving
> > > it alone here.
> > 
> > Hmm. Actually I'm not quite sure of the "good reason". Do you have any idea
on
> > it? I mean, I want to understand how tasak's fix
> > (https://codereview.chromium.org/808673005/) affects performance.
> 
> I want you and tasak to assure me that there isn't anything to be concerned
> with. I pointed out the mismatch here & left alone what seemed like an
> intentional const-ref mapping for this trait, as it seemed to be that way on
> purpose comparing to the other mappings surrounding it.

tasak@: Would you mind investigating it? You can probably revert the CL until
we're sure that it's a right change.

[sof, 2014-12-16 08:23:17]

On 2014/12/16 08:08:53, haraken wrote:
> On 2014/12/16 08:06:23, sof wrote:
> > On 2014/12/16 07:57:44, haraken wrote:
> > > On 2014/10/16 15:50:22, sof wrote:
> > > > Please take a look.
> > > > 
> > > > The problem with these "const auto&" range loops is due to having
> > > > HashTraits<Member<T>>::IteratorConstReferenceType map to T* with Oilpan,
> > which
> > > > leads to confusion when iterating a type like
> > > > 
> > > >    WillBeHeapHashSet<RefPtrWillBeMember<Widget>>
> > > > 
> > > > as the same hash trait type for RefPtr<> keeps the const reference as a
> > > > RefPtr<>.
> > > > 
> > > > I assume the Oilpan mapping of T* is that way for a good reason, and am
> > > leaving
> > > > it alone here.
> > > 
> > > Hmm. Actually I'm not quite sure of the "good reason". Do you have any
idea
> on
> > > it? I mean, I want to understand how tasak's fix
> > > (https://codereview.chromium.org/808673005/) affects performance.
> > 
> > I want you and tasak to assure me that there isn't anything to be concerned
> > with. I pointed out the mismatch here & left alone what seemed like an
> > intentional const-ref mapping for this trait, as it seemed to be that way on
> > purpose comparing to the other mappings surrounding it.
> 
> tasak@: Would you mind investigating it? You can probably revert the CL until
> we're sure that it's a right change.

My respect for the automagic collection traits machinery is probably too great
:) Erik might be able to shed light on whether or not there is anything deeper
going on here with that mapping.