Use MessageLoopProxy instead of MessageLoop to dispatch notifications in ObserverListThreadsafe.

Use MessageLoopProxy instead of MessageLoop to dispatch notifications in ObserverListThreadsafe.

This is nearly identical to the reverted r96013, with the modification that
std::map::insert isn't used.  Its invocation was complex (such that
it worked in some compilers but not in others) and the performance gain is
negligible in any use case I've seen in Chromium.

BUG=91589
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96076

[darin (slow to review), 2011-08-09 19:32:31]

LGTM

http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h
File base/observer_list_threadsafe.h (right):

http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h...
base/observer_list_threadsafe.h:100: observer_lists_[loop] = new
ObserverListContext(type_);
it is very surprising that insert did not work.  i'm pretty sure we use that
elsewhere.  what was the failure?

[adamk, 2011-08-09 20:33:40]

http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h
File base/observer_list_threadsafe.h (right):

http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h...
base/observer_list_threadsafe.h:100: observer_lists_[loop] = new
ObserverListContext(type_);
On 2011/08/09 19:32:31, darin wrote:
> it is very surprising that insert did not work.  i'm pretty sure we use that
> elsewhere.  what was the failure?

The failure was due to NULL not being "typed" enough.  I originally ran into
this when using std::make_pair(), and fixed it for all known configurations by
using std::map::value_type, but apparently that wasn't good enough for VS2010. 
If you'd like, I can _probably_ get it to compile with the addition of a
static_cast, but it makes the code even harder to read. Up to you.

[darin (slow to review), 2011-08-09 20:42:18]

On Tue, Aug 9, 2011 at 1:33 PM, <adamk@chromium.org> wrote:

>
> http://codereview.chromium.**org/7604006/diff/1/base/**
>
observer_list_threadsafe.h<http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h>
> File base/observer_list_threadsafe.**h (right):
>
> http://codereview.chromium.**org/7604006/diff/1/base/**
>
observer_list_threadsafe.h#**newcode100<http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h#newcode100>
> base/observer_list_threadsafe.**h:100: observer_lists_[loop] = new
> ObserverListContext(type_);
> On 2011/08/09 19:32:31, darin wrote:
>
>> it is very surprising that insert did not work.  i'm pretty sure we
>>
> use that
>
>> elsewhere.  what was the failure?
>>
>
> The failure was due to NULL not being "typed" enough.  I originally ran
> into this when using std::make_pair(), and fixed it for all known
> configurations by using std::map::value_type, but apparently that wasn't
> good enough for VS2010.  If you'd like, I can _probably_ get it to
> compile with the addition of a static_cast, but it makes the code even
> harder to read. Up to you.


I think the current code is fine.  I'm surprised the value_type typedef
didn't work.
-Darin


>
>
>
http://codereview.chromium.**org/7604006/<http://codereview.chromium.org/7604...
>

[adamk, 2011-08-09 21:13:00]

On Tue, Aug 9, 2011 at 1:42 PM, Darin Fisher <darin@chromium.org> wrote:
>
>
> On Tue, Aug 9, 2011 at 1:33 PM, <adamk@chromium.org> wrote:
>>
>>
>> http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h
>> File base/observer_list_threadsafe.h (right):
>>
>>
>>
http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h...
>> base/observer_list_threadsafe.h:100: observer_lists_[loop] = new
>> ObserverListContext(type_);
>> On 2011/08/09 19:32:31, darin wrote:
>>>
>>> it is very surprising that insert did not work.  i'm pretty sure we
>>
>> use that
>>>
>>> elsewhere.  what was the failure?
>>
>> The failure was due to NULL not being "typed" enough.  I originally ran
>> into this when using std::make_pair(), and fixed it for all known
>> configurations by using std::map::value_type, but apparently that wasn't
>> good enough for VS2010.  If you'd like, I can _probably_ get it to
>> compile with the addition of a static_cast, but it makes the code even
>> harder to read. Up to you.
>
> I think the current code is fine.  I'm surprised the value_type typedef
> didn't work.

Yeah, I was surprised too.  Here's the compiler output if you're interested:

http://build.chromium.org/p/chromium/builders/Win%20Builder%202010%20%28dbg%2...

> -Darin
>
>>
>> http://codereview.chromium.org/7604006/
>
>

[adamk, 2011-08-09 22:10:23]

On Tue, Aug 9, 2011 at 2:12 PM, Adam Klein <adamk@chromium.org> wrote:
> On Tue, Aug 9, 2011 at 1:42 PM, Darin Fisher <darin@chromium.org> wrote:
>>
>>
>> On Tue, Aug 9, 2011 at 1:33 PM, <adamk@chromium.org> wrote:
>>>
>>>
>>>
http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h
>>> File base/observer_list_threadsafe.h (right):
>>>
>>>
>>>
http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h...
>>> base/observer_list_threadsafe.h:100: observer_lists_[loop] = new
>>> ObserverListContext(type_);
>>> On 2011/08/09 19:32:31, darin wrote:
>>>>
>>>> it is very surprising that insert did not work.  i'm pretty sure we
>>>
>>> use that
>>>>
>>>> elsewhere.  what was the failure?
>>>
>>> The failure was due to NULL not being "typed" enough.  I originally ran
>>> into this when using std::make_pair(), and fixed it for all known
>>> configurations by using std::map::value_type, but apparently that wasn't
>>> good enough for VS2010.  If you'd like, I can _probably_ get it to
>>> compile with the addition of a static_cast, but it makes the code even
>>> harder to read. Up to you.
>>
>> I think the current code is fine.  I'm surprised the value_type typedef
>> didn't work.
>
> Yeah, I was surprised too.  Here's the compiler output if you're interested:
>
>
http://build.chromium.org/p/chromium/builders/Win%20Builder%202010%20%28dbg%2...

And from
http://stackoverflow.com/questions/4596499/how-to-specify-null-as-pointer-typ...,
sounds like it maybe be C++0x-related.

>> -Darin
>>
>>>
>>> http://codereview.chromium.org/7604006/
>>
>>
>

[darin (slow to review), 2011-08-09 22:21:52]

On Tue, Aug 9, 2011 at 3:10 PM, Adam Klein <adamk@chromium.org> wrote:

> On Tue, Aug 9, 2011 at 2:12 PM, Adam Klein <adamk@chromium.org> wrote:
> > On Tue, Aug 9, 2011 at 1:42 PM, Darin Fisher <darin@chromium.org> wrote:
> >>
> >>
> >> On Tue, Aug 9, 2011 at 1:33 PM, <adamk@chromium.org> wrote:
> >>>
> >>>
> >>>
> http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h
> >>> File base/observer_list_threadsafe.h (right):
> >>>
> >>>
> >>>
>
http://codereview.chromium.org/7604006/diff/1/base/observer_list_threadsafe.h...
> >>> base/observer_list_threadsafe.h:100: observer_lists_[loop] = new
> >>> ObserverListContext(type_);
> >>> On 2011/08/09 19:32:31, darin wrote:
> >>>>
> >>>> it is very surprising that insert did not work.  i'm pretty sure we
> >>>
> >>> use that
> >>>>
> >>>> elsewhere.  what was the failure?
> >>>
> >>> The failure was due to NULL not being "typed" enough.  I originally ran
> >>> into this when using std::make_pair(), and fixed it for all known
> >>> configurations by using std::map::value_type, but apparently that
> wasn't
> >>> good enough for VS2010.  If you'd like, I can _probably_ get it to
> >>> compile with the addition of a static_cast, but it makes the code even
> >>> harder to read. Up to you.
> >>
> >> I think the current code is fine.  I'm surprised the value_type typedef
> >> didn't work.
> >
> > Yeah, I was surprised too.  Here's the compiler output if you're
> interested:
> >
> >
>
http://build.chromium.org/p/chromium/builders/Win%20Builder%202010%20%28dbg%2...
>
> And from
>
http://stackoverflow.com/questions/4596499/how-to-specify-null-as-pointer-typ...
> ,
> sounds like it maybe be C++0x-related.
>
>
Ah, nullptr :-/



> >> -Darin
> >>
> >>>
> >>> http://codereview.chromium.org/7604006/
> >>
> >>
> >
>