Fix data race in ChannelAssociatedGroupController

Fix data race in ChannelAssociatedGroupController

Introduces an atomic value to guard access to a cached
TaskRunner which read from multiple threads but written
at some arbitrary time on the IPC thread inside Bind().

BUG=630835
R=yzshen@chromium.org

Committed: https://crrev.com/9098435cfcf0bb09325351beabbb27c8ad62632f
Cr-Commit-Position: refs/heads/master@{#407504}

[Ken Rockot(use gerrit already), 2016-07-23 13:05:52]

Please take a look. I'm not too happy about this, so please let me know if you
have a better idea :)

The reason for thread_task_runner_ to exist is that BrowserThreadTaskRunner
incorrectly reports that it doesn't run tasks on its own thread if called during
shutdown, e.g. within a WillDestroyCurrentMessageLoop. I added this to cache the
default task runner of the IPC thread, but forgot to guard access to it. WDYT?

[yzshen1, 2016-07-25 16:33:59]

On 2016/07/23 13:05:52, Ken Rockot wrote:
> Please take a look. I'm not too happy about this, so please let me know if you
> have a better idea :)
> 
> The reason for thread_task_runner_ to exist is that BrowserThreadTaskRunner
> incorrectly reports that it doesn't run tasks on its own thread if called
during
> shutdown, e.g. within a WillDestroyCurrentMessageLoop. I added this to cache
the
> default task runner of the IPC thread, but forgot to guard access to it. WDYT?

Yeah, it seems a little sad to do this.
I feel it may be better to fix BrowserThreadTaskRunner's behavior.
Besides, I guess the usage of the atomic value is to ensure memory visibility,
right? I would prefer a lock since it is easier to understand by average
developers. But I think you probably have considered this alternative.

LGTM

[Ken Rockot(use gerrit already), 2016-07-25 16:38:24]

Thanks - yeah I didn't want to add another lock since I expect it would be
frequently contended (no data to back this up of course ;)). Besides I would
like this to be a temporary state and I agree we should fix
BrowserThreadTaskRunner. I will file a bug for that and land this for now.

[Ken Rockot(use gerrit already), 2016-07-25 16:38:28]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-07-25 16:38:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 17:36:40]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-25 17:39:15]

Description was changed from

==========
Fix data race in ChannelAssociatedGroupController

Introduces an atomic value to guard access to a cached
TaskRunner which read from multiple threads but written
at some arbitrary time on the IPC thread inside Bind().

BUG=630835
R=yzshen@chromium.org
==========

to

==========
Fix data race in ChannelAssociatedGroupController

Introduces an atomic value to guard access to a cached
TaskRunner which read from multiple threads but written
at some arbitrary time on the IPC thread inside Bind().

BUG=630835
R=yzshen@chromium.org

Committed: https://crrev.com/9098435cfcf0bb09325351beabbb27c8ad62632f
Cr-Commit-Position: refs/heads/master@{#407504}
==========

[commit-bot: I haz the power, 2016-07-25 17:39:16]

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