[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

When a user signs out and Sync starts to shut down, we immediately
clear the LocalDeviceInfoProvider. Many methods in DeviceInfoSyncBridge
depend on being able to access information in the provider, and can
crash at this point. The bridge should be disabled momentarily, but
this will require threads hops UI->Sync->UI. Meanwhile, various other
posted tasks can call into the bridge and hit this race condition.

We were previously guarding against this scenario when the pulse timer
would fire, but we were not guarding against processor's Merge/Apply
calls or ModelTypeStore async initialization. I've added nullptr checks
to all async entry points that mostly no-op when they detect this
scenario.

One slight hiccup is that the bridge currently leaves the pulse timer
running even when disabled. My intention is that this change will be
merged back into M56 so I wanted to keep this CL as small as possible.
When crbug.com/672600 is addressed I think it will make the most sense
to disable the pulse timer while disabled, since it should always be
re-enabled with an accurate timer when things are working again.

Additionally, some state tracking such as has_provider_initialized_
should probably just be removed at this point. This should be caught in
crbug.com/659263

BUG=672534
Committed: https://crrev.com/24268f797538e6add4338602ff1ebd78affe2648
Cr-Commit-Position: refs/heads/master@{#437902}

[skym, 2016-12-09 20:40:40]

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

[commit-bot: I haz the power, 2016-12-09 20:41:52]

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

[skym, 2016-12-09 20:48:31]

Description was changed from

==========
[Sync] Activly gaurd against provider being cleared in DeviceInfoSyncBridge.

BUG=672534
==========

to

==========
[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

When a user signs out and Sync starts to shut down, we immediately
clear the LocalDeviceInfoProvider. Many methods in DeviceInfoSyncBridge
depend on being able to access information in the provider, and can
crash at this point. The bridge should be disabled momentarily, but
this will require threads hops UI->Sync->UI. Meanwhile, various other
posted tasks can call into the bridge and hit this race condition.

We were previously guarding against this scenario when the pulse timer
would fire, but we were not guarding against processor's Merge/Apply
calls or ModelTypeStore async initialization. I've added nullptr checks
to all async entry points that mostly no-op when they detect this
scenario.

One slight hiccup is that the bridge currently leaves the pulse timer
running even when disabled. My intention is that this change will be
merged back into M56 so I wanted to keep this CL as small as possible.
When crbug.com/672600 is addressed I think it will make the most sense
to disable the pulse timer while disabled, since it should always be
re-enabled with an accurate timer when things are working again.

Additionally, some state tracking such as has_provider_initialized_
should probably just be removed at this point. This should be caught in
crbug.com/659263

BUG=672534
==========

[skym, 2016-12-09 20:48:31]

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

[skym, 2016-12-09 20:48:53]

PTAL

[commit-bot: I haz the power, 2016-12-09 22:42:33]

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

[commit-bot: I haz the power, 2016-12-09 22:42:34]

Dry run: This issue passed the CQ dry run.

[maxbogue, 2016-12-10 00:28:29]

lgtm, but I think we should really consider forcing consumers of DeviceInfo to
cache it instead of all of them needing to go through this sort of thing.

[skym, 2016-12-12 16:59:43]

The CQ bit was checked by skym@chromium.org

[commit-bot: I haz the power, 2016-12-12 17:00:07]

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-12 18:22:10]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481561983596180,
"parent_rev": "f6b744608be72407dba2c4fb1b8e162be0e49dc4", "commit_rev":
"bc3dba2264655162bd265f30e35e2f9828dedb4b"}

[commit-bot: I haz the power, 2016-12-12 18:22:38]

Description was changed from

==========
[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

When a user signs out and Sync starts to shut down, we immediately
clear the LocalDeviceInfoProvider. Many methods in DeviceInfoSyncBridge
depend on being able to access information in the provider, and can
crash at this point. The bridge should be disabled momentarily, but
this will require threads hops UI->Sync->UI. Meanwhile, various other
posted tasks can call into the bridge and hit this race condition.

We were previously guarding against this scenario when the pulse timer
would fire, but we were not guarding against processor's Merge/Apply
calls or ModelTypeStore async initialization. I've added nullptr checks
to all async entry points that mostly no-op when they detect this
scenario.

One slight hiccup is that the bridge currently leaves the pulse timer
running even when disabled. My intention is that this change will be
merged back into M56 so I wanted to keep this CL as small as possible.
When crbug.com/672600 is addressed I think it will make the most sense
to disable the pulse timer while disabled, since it should always be
re-enabled with an accurate timer when things are working again.

Additionally, some state tracking such as has_provider_initialized_
should probably just be removed at this point. This should be caught in
crbug.com/659263

BUG=672534
==========

to

==========
[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

When a user signs out and Sync starts to shut down, we immediately
clear the LocalDeviceInfoProvider. Many methods in DeviceInfoSyncBridge
depend on being able to access information in the provider, and can
crash at this point. The bridge should be disabled momentarily, but
this will require threads hops UI->Sync->UI. Meanwhile, various other
posted tasks can call into the bridge and hit this race condition.

We were previously guarding against this scenario when the pulse timer
would fire, but we were not guarding against processor's Merge/Apply
calls or ModelTypeStore async initialization. I've added nullptr checks
to all async entry points that mostly no-op when they detect this
scenario.

One slight hiccup is that the bridge currently leaves the pulse timer
running even when disabled. My intention is that this change will be
merged back into M56 so I wanted to keep this CL as small as possible.
When crbug.com/672600 is addressed I think it will make the most sense
to disable the pulse timer while disabled, since it should always be
re-enabled with an accurate timer when things are working again.

Additionally, some state tracking such as has_provider_initialized_
should probably just be removed at this point. This should be caught in
crbug.com/659263

BUG=672534

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

[commit-bot: I haz the power, 2016-12-12 18:22:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-12 18:35:14]

Description was changed from

==========
[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

When a user signs out and Sync starts to shut down, we immediately
clear the LocalDeviceInfoProvider. Many methods in DeviceInfoSyncBridge
depend on being able to access information in the provider, and can
crash at this point. The bridge should be disabled momentarily, but
this will require threads hops UI->Sync->UI. Meanwhile, various other
posted tasks can call into the bridge and hit this race condition.

We were previously guarding against this scenario when the pulse timer
would fire, but we were not guarding against processor's Merge/Apply
calls or ModelTypeStore async initialization. I've added nullptr checks
to all async entry points that mostly no-op when they detect this
scenario.

One slight hiccup is that the bridge currently leaves the pulse timer
running even when disabled. My intention is that this change will be
merged back into M56 so I wanted to keep this CL as small as possible.
When crbug.com/672600 is addressed I think it will make the most sense
to disable the pulse timer while disabled, since it should always be
re-enabled with an accurate timer when things are working again.

Additionally, some state tracking such as has_provider_initialized_
should probably just be removed at this point. This should be caught in
crbug.com/659263

BUG=672534

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

to

==========
[Sync] Actively guard against provider being cleared in DeviceInfoSyncBridge.

When a user signs out and Sync starts to shut down, we immediately
clear the LocalDeviceInfoProvider. Many methods in DeviceInfoSyncBridge
depend on being able to access information in the provider, and can
crash at this point. The bridge should be disabled momentarily, but
this will require threads hops UI->Sync->UI. Meanwhile, various other
posted tasks can call into the bridge and hit this race condition.

We were previously guarding against this scenario when the pulse timer
would fire, but we were not guarding against processor's Merge/Apply
calls or ModelTypeStore async initialization. I've added nullptr checks
to all async entry points that mostly no-op when they detect this
scenario.

One slight hiccup is that the bridge currently leaves the pulse timer
running even when disabled. My intention is that this change will be
merged back into M56 so I wanted to keep this CL as small as possible.
When crbug.com/672600 is addressed I think it will make the most sense
to disable the pulse timer while disabled, since it should always be
re-enabled with an accurate timer when things are working again.

Additionally, some state tracking such as has_provider_initialized_
should probably just be removed at this point. This should be caught in
crbug.com/659263

BUG=672534

Committed: https://crrev.com/24268f797538e6add4338602ff1ebd78affe2648
Cr-Commit-Position: refs/heads/master@{#437902}
==========

[commit-bot: I haz the power, 2016-12-12 18:35:15]

Patchset 2 (id:??) landed as
https://crrev.com/24268f797538e6add4338602ff1ebd78affe2648
Cr-Commit-Position: refs/heads/master@{#437902}