[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=669967
Committed: https://crrev.com/75b2278e77eda3ac01262a16ca45151ab8253e06
Cr-Commit-Position: refs/heads/master@{#435501}

[maxbogue, 2016-11-30 01:46:51]

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

[commit-bot: I haz the power, 2016-11-30 01:47:50]

Dry run: 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-11-30 03:49:45]

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

[commit-bot: I haz the power, 2016-11-30 03:49:46]

Dry run: Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[maxbogue, 2016-11-30 17:07:29]

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

[commit-bot: I haz the power, 2016-11-30 17:07:48]

Dry run: 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-11-30 17:56:18]

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

[commit-bot: I haz the power, 2016-11-30 17:56:19]

Dry run: This issue passed the CQ dry run.

[maxbogue, 2016-11-30 18:02:17]

Description was changed from

==========
[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=
==========

to

==========
[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=669967
==========

[maxbogue, 2016-11-30 18:02:29]

maxbogue@chromium.org changed reviewers:
+ skym@chromium.org

[maxbogue, 2016-11-30 18:02:29]

Sky, PTAL!

[skym, 2016-11-30 18:45:23]

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_core.cc (left):

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_core.cc:144: if (!sync_loop_)
What makes us think that removing all these if (!sync_loop_) return; checks is
safe?

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_core.cc (right):

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_core.cc:432:
DCHECK(sync_task_runner_->BelongsToCurrentThread());
Seems like this class could get away with a ThreadCheckers and
SequencedTaskRunnerHandle::Get() instead. We talked offline, you said that you
wanted the interfaces to look the same. Okay, but lets just throw away the
SingleThreadTaskRunner.

[maxbogue, 2016-11-30 20:03:01]

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

[commit-bot: I haz the power, 2016-11-30 20:03:36]

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

[maxbogue, 2016-11-30 20:05:06]

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_core.cc (left):

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_core.cc:144: if (!sync_loop_)
On 2016/11/30 18:45:23, skym wrote:
> What makes us think that removing all these if (!sync_loop_) return; checks is
> safe?

It was never nulled out and it was set in DoInitialize (the first thing that's
ever called on this class).

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
File components/sync/driver/glue/sync_backend_host_core.cc (right):

https://codereview.chromium.org/2538023002/diff/1/components/sync/driver/glue...
components/sync/driver/glue/sync_backend_host_core.cc:432:
DCHECK(sync_task_runner_->BelongsToCurrentThread());
On 2016/11/30 18:45:23, skym wrote:
> Seems like this class could get away with a ThreadCheckers and
> SequencedTaskRunnerHandle::Get() instead. We talked offline, you said that you
> wanted the interfaces to look the same. Okay, but lets just throw away the
> SingleThreadTaskRunner.

Done. Except I used ThreadTaskRunnerHandle::Get() because it's important that
the sync thread stuff is a thread and not just a sequence.

[commit-bot: I haz the power, 2016-11-30 20:16:14]

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

[commit-bot: I haz the power, 2016-11-30 20:16:16]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[maxbogue, 2016-11-30 20:49:00]

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

[commit-bot: I haz the power, 2016-11-30 20:49:35]

Dry run: 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-11-30 22:20:53]

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

[commit-bot: I haz the power, 2016-11-30 22:20:54]

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...)

[skym, 2016-11-30 22:39:34]

lgtm

[maxbogue, 2016-11-30 23:53:50]

The CQ bit was checked by maxbogue@chromium.org

[maxbogue, 2016-11-30 23:53:50]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2538023002/#ps60001
(title: "Remove thread check in destructor.")

[commit-bot: I haz the power, 2016-11-30 23:54:12]

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-01 00:44:26]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480550030404690,
"parent_rev": "66f46d076f8011652bb9c9105124d7587a6d5b23", "commit_rev":
"954ffe863159ef840dd3e260ff4c0ea2126088ff"}

[commit-bot: I haz the power, 2016-12-01 00:44:50]

Description was changed from

==========
[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=669967
==========

to

==========
[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=669967
==========

[commit-bot: I haz the power, 2016-12-01 00:44:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-01 00:48:56]

Description was changed from

==========
[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=669967
==========

to

==========
[Sync] Pass a TaskRunner into SBHI/SBHC, not a thread or message loop.

BUG=669967

Committed: https://crrev.com/75b2278e77eda3ac01262a16ca45151ab8253e06
Cr-Commit-Position: refs/heads/master@{#435501}
==========

[commit-bot: I haz the power, 2016-12-01 00:48:57]

Patchset 4 (id:??) landed as
https://crrev.com/75b2278e77eda3ac01262a16ca45151ab8253e06
Cr-Commit-Position: refs/heads/master@{#435501}