[tracing] Fix races in TraceLog's EnabledStateObserver

[tracing] Fix races in TraceLog's EnabledStateObserver

This CL makes TraceLog Add/RemoveEnabledStateObserver thread safe
as observers can be registered on different threads.
Furthermore this CL fixes another race in
ChildMemoryDumpManagerDelegateImpl, ensuring that the check to
post the memory dump request on the right thread is itself
thread-safe.

BUG=530578
Committed: https://crrev.com/74188c63d45324feb6a3abbc36fff1affd322801
Cr-Commit-Position: refs/heads/master@{#349176}

[Primiano Tucci (use gerrit), 2015-09-16 16:58:43]

primiano@chromium.org changed reviewers:
+ dsinclair@chromium.org

[dsinclair, 2015-09-16 17:14:55]

https://codereview.chromium.org/1348613006/diff/1/components/tracing/child_me...
File components/tracing/child_memory_dump_manager_delegate_impl.cc (right):

https://codereview.chromium.org/1348613006/diff/1/components/tracing/child_me...
components/tracing/child_memory_dump_manager_delegate_impl.cc:50:
base::trace_event::MemoryDumpManager::GetInstance()->Initialize(
Why does the initialize call move from the constructor to the destructor?

[Primiano Tucci (use gerrit), 2015-09-16 17:46:20]

https://codereview.chromium.org/1348613006/diff/1/components/tracing/child_me...
File components/tracing/child_memory_dump_manager_delegate_impl.cc (right):

https://codereview.chromium.org/1348613006/diff/1/components/tracing/child_me...
components/tracing/child_memory_dump_manager_delegate_impl.cc:50:
base::trace_event::MemoryDumpManager::GetInstance()->Initialize(
On 2015/09/16 17:14:55, dsinclair wrote:
> Why does the initialize call move from the constructor to the destructor?

Uh? this is not the dtor. This is the SetChildTraceMessageFilter.
Rationale is: there is no point initializing the manager before we have set the
filter. If we do that, we know upfront that we'll NACK requests until we get
filter set, so this is to avoid any further issues.
In other words: it should be safe leaving it where it is, but there is no real
point anticipating the initialization there.
This is the point where the MDM + CMDMDI become useful.

[dsinclair, 2015-09-16 17:53:03]

lgtm

https://codereview.chromium.org/1348613006/diff/1/components/tracing/child_me...
File components/tracing/child_memory_dump_manager_delegate_impl.cc (right):

https://codereview.chromium.org/1348613006/diff/1/components/tracing/child_me...
components/tracing/child_memory_dump_manager_delegate_impl.cc:50:
base::trace_event::MemoryDumpManager::GetInstance()->Initialize(
On 2015/09/16 17:46:20, Primiano Tucci wrote:
> On 2015/09/16 17:14:55, dsinclair wrote:
> > Why does the initialize call move from the constructor to the destructor?
> 
> Uh? this is not the dtor. This is the SetChildTraceMessageFilter.
> Rationale is: there is no point initializing the manager before we have set
the
> filter. If we do that, we know upfront that we'll NACK requests until we get
> filter set, so this is to avoid any further issues.
> In other words: it should be safe leaving it where it is, but there is no real
> point anticipating the initialization there.
> This is the point where the MDM + CMDMDI become useful.


Doh, skipped line 36 when I scanned up and just saw 34. ECANTREAD.

[Primiano Tucci (use gerrit), 2015-09-16 18:05:30]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2015-09-16 18:06:30]

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

[commit-bot: I haz the power, 2015-09-16 18:59:14]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-09-16 19:00:06]

Patchset 1 (id:??) landed as
https://crrev.com/74188c63d45324feb6a3abbc36fff1affd322801
Cr-Commit-Position: refs/heads/master@{#349176}

[commit-bot: I haz the power, 2015-09-23 12:58:41]

Patchset 1 (id:??) landed as
https://crrev.com/74188c63d45324feb6a3abbc36fff1affd322801
Cr-Commit-Position: refs/heads/master@{#349176}