Fix data race in RenderThreadImplBrowserTest

Fix data race in RenderThreadImplBrowserTest

Use ThreadTaskRunnerHandle instead of BrowserThread::GetTaskRunnerForThread.
This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl's TaskRunner facade does not anticipate.

Also opportunistically corrects some non-const ref arguments in
RenderThreadImpl.

BUG=672812
Committed: https://crrev.com/97a8683a8e2243efcdf13273a4037d5905ab75dd
Cr-Commit-Position: refs/heads/master@{#437755}

[Ken Rockot(use gerrit already), 2016-12-09 19:09:52]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 19:10:40]

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

[Ken Rockot(use gerrit already), 2016-12-09 19:25:21]

Description was changed from

==========
Fix data race in RenderThreadImplBrowserTest

Uses a ThreadTaskRunnerHandle instead of the BrowserThread's TaskRunner
implementation.

BUG=672812
==========

to

==========
Fix data race in RenderThreadImplBrowserTest

Uses the ThreadTaskRunnerHandle instead of the BrowserThread's TaskRunner.

This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl does not anticipate.

BUG=672812
==========

[Ken Rockot(use gerrit already), 2016-12-09 19:25:22]

rockot@chromium.org changed reviewers:
+ jam@chromium.org

[commit-bot: I haz the power, 2016-12-09 19:43:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 19:43:31]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Ken Rockot(use gerrit already), 2016-12-09 21:12:15]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 21:13:16]

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

[Ken Rockot(use gerrit already), 2016-12-09 21:14:27]

Description was changed from

==========
Fix data race in RenderThreadImplBrowserTest

Uses the ThreadTaskRunnerHandle instead of the BrowserThread's TaskRunner.

This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl does not anticipate.

BUG=672812
==========

to

==========
Fix data race in RenderThreadImplBrowserTest

This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl does not anticipate.

Because it was already incorrect to be using ChannelProxy on the same thread
as its Channel, this changes the TestBrowserThreadBundle to use REAL_IO_THREAD
instead of IO_MAINLOOP.

As one further consequence of running a real IO thread, it also then becomes
possible to initialize the ServiceManagerContext before the SequenceWorkerPool
is initialized, due to yet another subtlety of this particular test fixture.

So this shuffles some stuff around to avoid that.

Finally, fixes some non-const ref parameter usage in RenderThreadImpl.

BUG=672812
==========

[Ken Rockot(use gerrit already), 2016-12-09 21:15:37]

I have half a mind to just delete these tests. At this point the test fixture
setup code is probably more complex and fragile than the behavior it's being
used to test.

[commit-bot: I haz the power, 2016-12-09 23:15:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 23:15:22]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jam, 2016-12-10 00:07:32]

lgtm

[Ken Rockot(use gerrit already), 2016-12-10 04:01:00]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-10 04:01:20]

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

[Ken Rockot(use gerrit already), 2016-12-10 04:02:07]

Description was changed from

==========
Fix data race in RenderThreadImplBrowserTest

This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl does not anticipate.

Because it was already incorrect to be using ChannelProxy on the same thread
as its Channel, this changes the TestBrowserThreadBundle to use REAL_IO_THREAD
instead of IO_MAINLOOP.

As one further consequence of running a real IO thread, it also then becomes
possible to initialize the ServiceManagerContext before the SequenceWorkerPool
is initialized, due to yet another subtlety of this particular test fixture.

So this shuffles some stuff around to avoid that.

Finally, fixes some non-const ref parameter usage in RenderThreadImpl.

BUG=672812
==========

to

==========
Fix data race in RenderThreadImplBrowserTest

Use ThreadTaskRunnerHandle instead of BrowserThread::GetTaskRunnerForThread.
This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl's TaskRunner facade does not anticipate.

Also opportunistically corrects some non-const ref arguments in
RenderThreadImpl.

BUG=672812
==========

[Ken Rockot(use gerrit already), 2016-12-10 04:02:20]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-12-10 04:02:21]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-12-10 04:02:21]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2568543002/#ps40001
(title: ".")

[commit-bot: I haz the power, 2016-12-10 04:02:31]

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-12-10 04:53:11]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481342541440180,
"parent_rev": "767996a21a472ca1a74c89a63084e4c584343481", "commit_rev":
"e3916da3937d887e5d0b2ea9f12401a39415196f"}

[commit-bot: I haz the power, 2016-12-10 04:53:37]

Description was changed from

==========
Fix data race in RenderThreadImplBrowserTest

Use ThreadTaskRunnerHandle instead of BrowserThread::GetTaskRunnerForThread.
This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl's TaskRunner facade does not anticipate.

Also opportunistically corrects some non-const ref arguments in
RenderThreadImpl.

BUG=672812
==========

to

==========
Fix data race in RenderThreadImplBrowserTest

Use ThreadTaskRunnerHandle instead of BrowserThread::GetTaskRunnerForThread.
This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl's TaskRunner facade does not anticipate.

Also opportunistically corrects some non-const ref arguments in
RenderThreadImpl.

BUG=672812

Review-Url: https://codereview.chromium.org/2568543002
==========

[commit-bot: I haz the power, 2016-12-10 04:53:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-12 15:06:29]

Description was changed from

==========
Fix data race in RenderThreadImplBrowserTest

Use ThreadTaskRunnerHandle instead of BrowserThread::GetTaskRunnerForThread.
This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl's TaskRunner facade does not anticipate.

Also opportunistically corrects some non-const ref arguments in
RenderThreadImpl.

BUG=672812

Review-Url: https://codereview.chromium.org/2568543002
==========

to

==========
Fix data race in RenderThreadImplBrowserTest

Use ThreadTaskRunnerHandle instead of BrowserThread::GetTaskRunnerForThread.
This avoids a data race in shutdown due to the fact that RendererScheduler
(used on a BrowserThread in this test) tramples the MessageLoop's TaskRunner
in a manner BrowserThreadImpl's TaskRunner facade does not anticipate.

Also opportunistically corrects some non-const ref arguments in
RenderThreadImpl.

BUG=672812

Committed: https://crrev.com/97a8683a8e2243efcdf13273a4037d5905ab75dd
Cr-Commit-Position: refs/heads/master@{#437755}
==========

[commit-bot: I haz the power, 2016-12-12 15:06:30]

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