base: Stop passing a non-const ref to ObserverListBase::Iterator.

base: Stop passing a non-const ref to ObserverListBase::Iterator.

Pass a pointer instead, as per the style guide. Also makes the
constructor explicit (all callsites already are explicit).

BUG=464816
TBR=blundell@chromium.org,agl,dmichael,garykac

Committed: https://crrev.com/4ef352de3f24142d38b7d11fe3f9f637b43ee834
Cr-Commit-Position: refs/heads/master@{#319674}

[danakj, 2015-03-06 21:41:40]

danakj@chromium.org changed reviewers:
+ avi@chromium.org

[danakj, 2015-03-06 21:42:06]

danakj@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-03-06 21:50:05]

lgtm

[Avi (use Gerrit), 2015-03-06 23:58:49]

On 2015/03/06 21:50:05, Nico wrote:
> lgtm

You mean you can't make it a const reference? Observer list iterators modify the
observer list?! ಠ_ಠ

[danakj, 2015-03-07 00:00:08]

On 2015/03/06 23:58:49, Avi wrote:
> On 2015/03/06 21:50:05, Nico wrote:
> > lgtm
> 
> You mean you can't make it a const reference? Observer list iterators modify
the
> observer list?! ಠ_ಠ

I think it's a matter of observers with non-const methods being called.. let me
try it out again though..

[Avi (use Gerrit), 2015-03-07 00:00:27]

On 2015/03/06 23:58:49, Avi wrote:
> On 2015/03/06 21:50:05, Nico wrote:
> > lgtm
> 
> You mean you can't make it a const reference? Observer list iterators modify
the
> observer list?! ಠ_ಠ

And now that I have that out of my system, LGTM. Am not thrilled here.

[danakj, 2015-03-07 00:10:09]

On 2015/03/07 00:00:27, Avi wrote:
> On 2015/03/06 23:58:49, Avi wrote:
> > On 2015/03/06 21:50:05, Nico wrote:
> > > lgtm
> > 
> > You mean you can't make it a const reference? Observer list iterators modify
> the
> > observer list?! ಠ_ಠ
> 
> And now that I have that out of my system, LGTM. Am not thrilled here.

Oh, the iterator wants a weakptr, but AsWeakPtr is not const. That's at least
one problem. I could make a mutable trick to keep things const. Is that a good
idea to you?

[Avi (use Gerrit), 2015-03-07 00:14:11]

On 2015/03/07 00:10:09, danakj wrote:
> On 2015/03/07 00:00:27, Avi wrote:
> > On 2015/03/06 23:58:49, Avi wrote:
> > > On 2015/03/06 21:50:05, Nico wrote:
> > > > lgtm
> > > 
> > > You mean you can't make it a const reference? Observer list iterators
modify
> > the
> > > observer list?! ಠ_ಠ
> > 
> > And now that I have that out of my system, LGTM. Am not thrilled here.
> 
> Oh, the iterator wants a weakptr, but AsWeakPtr is not const. That's at least
> one problem. I could make a mutable trick to keep things const. Is that a good
> idea to you?

Well, the observer list also mutates an iterator count, and changes itself when
the last iterator detaches. I don't think it's worth the effort going down that
road. This change is fine.

[Nico, 2015-03-07 00:17:57]

Maybe there could be a const override of AsWeakPtr() that returns a
WeakPtr<const T>? Not sure if that would work though

[Nico, 2015-03-07 00:18:48]

(but i think this is fine as-is too)

On Fri, Mar 6, 2015 at 4:17 PM, <thakis@chromium.org> wrote:

> Maybe there could be a const override of AsWeakPtr() that returns a
> WeakPtr<const T>? Not sure if that would work though
>
> https://codereview.chromium.org/982413002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-03-07 00:22:42]

On Fri, Mar 6, 2015 at 4:14 PM, <avi@chromium.org> wrote:

> On 2015/03/07 00:10:09, danakj wrote:
>
>> On 2015/03/07 00:00:27, Avi wrote:
>> > On 2015/03/06 23:58:49, Avi wrote:
>> > > On 2015/03/06 21:50:05, Nico wrote:
>> > > > lgtm
>> > >
>> > > You mean you can't make it a const reference? Observer list iterators
>>
> modify
>
>> > the
>> > > observer list?! ಠ_ಠ
>> >
>> > And now that I have that out of my system, LGTM. Am not thrilled here.
>>
>
>  Oh, the iterator wants a weakptr, but AsWeakPtr is not const. That's at
>> least
>> one problem. I could make a mutable trick to keep things const. Is that a
>> good
>> idea to you?
>>
>
> Well, the observer list also mutates an iterator count, and changes itself
> when
> the last iterator detaches. I don't think it's worth the effort going down
> that
> road. This change is fine.
>

Cutely, the Iterator goes through the list_ WeakPtr for everything else,
which is non-const, so it's a really small change.. but of course the const
is totally a lie since it just turns it into a non-const pointer as the
first thing.

So, I guess the answer to your angry question is, as you say the observer
list iterators do fundamentally modify the observer list. I imagine this
was changed since you last looked at it or something. :)

OK we'll stick with this.


>
> https://codereview.chromium.org/982413002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Avi (use Gerrit), 2015-03-07 00:26:40]

On 2015/03/07 00:22:42, danakj wrote:
> So, I guess the answer to your angry question

To be clear, the ಠ_ಠ was intended to the ObserverList, not you, Dana. You're
cool; the ObserverList is... exceedingly clever. :(

[danakj, 2015-03-07 00:31:00]

On Fri, Mar 6, 2015 at 4:26 PM, <avi@chromium.org> wrote:

> On 2015/03/07 00:22:42, danakj wrote:
>
>> So, I guess the answer to your angry question
>>
>
> To be clear, the ಠ_ಠ was intended to the ObserverList, not you, Dana.
> You're
> cool; the ObserverList is... exceedingly clever. :(
>

haha, ya i figured ^_^ i was hoping i could save the day tho.


>
> https://codereview.chromium.org/982413002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-03-09 16:19:48]

danakj@chromium.org changed reviewers:
+ agl@chromium.org, blundell@chromium.org, dmichael@chromium.org,
garykac@chromium.org

[danakj, 2015-03-09 16:20:08]

The rest of the changes are purely mechanical, TBRing.

[danakj, 2015-03-09 16:20:13]

The CQ bit was checked by danakj@chromium.org

[danakj, 2015-03-09 16:20:14]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
avi@chromium.org
Link to the patchset: https://codereview.chromium.org/982413002/#ps40001 (title:
"observerref: fixwindowsbuild")

[commit-bot: I haz the power, 2015-03-09 16:20:45]

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

[garykac, 2015-03-09 16:38:52]

remoting lgtm

[dmichael (off chromium), 2015-03-09 16:44:31]

ppapi lgtm

[commit-bot: I haz the power, 2015-03-09 17:46:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-09 17:46:49]

Patchset 3 (id:??) landed as
https://crrev.com/4ef352de3f24142d38b7d11fe3f9f637b43ee834
Cr-Commit-Position: refs/heads/master@{#319674}