Change base class of V8SamplingProfiler to AsyncEnabledStateObserver.

Change base class of V8SamplingProfiler to AsyncEnabledStateObserver.

Currently V8SamplingProfiler is a subclass of EnabledStateObserver, however
EnabledStateObserver is not thread safe becuase OnTraceLog{Dis,En}abled get
called in different thread which will become a use-after-free case when observer
gets deleted in another thread.

This patch changes base class of V8SamplingProfiler to AsyncEnabledStateObserver
since AsyncEnabledStateObserver will post these two calls to the same thread
where observer was added.

BUG=611027
Committed: https://crrev.com/3d1573d4df84687881d4cfe8e8dd6391b99f28e2
Cr-Commit-Position: refs/heads/master@{#393260}

[lpy, 2016-05-11 21:57:58]

lpy@chromium.org changed reviewers:
+ alph@chromium.org

[lpy, 2016-05-11 21:57:59]

PTAL.

[alph, 2016-05-11 22:39:49]

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

[alph, 2016-05-11 22:39:51]

https://codereview.chromium.org/1967793004/diff/20001/content/renderer/devtoo...
File content/renderer/devtools/v8_sampling_profiler.cc (right):

https://codereview.chromium.org/1967793004/diff/20001/content/renderer/devtoo...
content/renderer/devtools/v8_sampling_profiler.cc:629:
task_runner_->PostTask(FROM_HERE,
Why do we need to post another task? The async state observer should already be
running on the proper thread.

https://codereview.chromium.org/1967793004/diff/20001/content/renderer/devtoo...
content/renderer/devtools/v8_sampling_profiler.cc:635:
task_runner_->PostTask(FROM_HERE,
ditto

[lpy, 2016-05-11 23:01:52]

Thanks, I updated the CL.

https://codereview.chromium.org/1967793004/diff/20001/content/renderer/devtoo...
File content/renderer/devtools/v8_sampling_profiler.cc (right):

https://codereview.chromium.org/1967793004/diff/20001/content/renderer/devtoo...
content/renderer/devtools/v8_sampling_profiler.cc:629:
task_runner_->PostTask(FROM_HERE,
On 2016/05/11 22:39:51, alph wrote:
> Why do we need to post another task? The async state observer should already
be
> running on the proper thread.

Done.

https://codereview.chromium.org/1967793004/diff/20001/content/renderer/devtoo...
content/renderer/devtools/v8_sampling_profiler.cc:635:
task_runner_->PostTask(FROM_HERE,
On 2016/05/11 22:39:51, alph wrote:
> ditto

Done.

[alph, 2016-05-11 23:29:33]

lgtm

[lpy, 2016-05-12 15:55:51]

The CQ bit was checked by lpy@chromium.org

[commit-bot: I haz the power, 2016-05-12 15:56:09]

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

[commit-bot: I haz the power, 2016-05-12 16:00:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-12 16:01:22]

Description was changed from

==========
Change base class of V8SamplingProfiler to AsyncEnabledStateObserver.

Currently V8SamplingProfiler is a subclass of EnabledStateObserver, however
EnabledStateObserver is not thread safe becuase OnTraceLog{Dis,En}abled get
called in different thread which will become a use-after-free case when observer
gets deleted in another thread.

This patch changes base class of V8SamplingProfiler to AsyncEnabledStateObserver
since AsyncEnabledStateObserver will post these two calls to the same thread
where observer was added.

BUG=611027
==========

to

==========
Change base class of V8SamplingProfiler to AsyncEnabledStateObserver.

Currently V8SamplingProfiler is a subclass of EnabledStateObserver, however
EnabledStateObserver is not thread safe becuase OnTraceLog{Dis,En}abled get
called in different thread which will become a use-after-free case when observer
gets deleted in another thread.

This patch changes base class of V8SamplingProfiler to AsyncEnabledStateObserver
since AsyncEnabledStateObserver will post these two calls to the same thread
where observer was added.

BUG=611027

Committed: https://crrev.com/3d1573d4df84687881d4cfe8e8dd6391b99f28e2
Cr-Commit-Position: refs/heads/master@{#393260}
==========

[commit-bot: I haz the power, 2016-05-12 16:01:23]

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

[Primiano Tucci (use gerrit), 2016-05-13 12:59:12]

non-owner LGTM. Maybe you can add a ThreadChecker to document/ensure it gets
called on the right thread.
Not strong on this suggestion, I'll leave it to alph.

[lpy, 2016-05-13 20:24:58]

On 2016/05/13 12:59:12, Primiano Tucci wrote:
> non-owner LGTM. Maybe you can add a ThreadChecker to document/ensure it gets
> called on the right thread.
> Not strong on this suggestion, I'll leave it to alph.

Thanks for the advice, sounds good to me.