Fix crash during sync load of the signin profile

Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device:
[FATAL:device_cloud_policy_manager_chromeos.cc(252)] Check
failed: signin_profile_forwarding_schema_registry_.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen on an enrolled
    device and verify that automatic restart works

Review-Url: https://codereview.chromium.org/2729093002
Cr-Commit-Position: refs/heads/master@{#454627}
Committed: https://chromium.googlesource.com/chromium/src/+/9bae7d67f2a11bc706b9becb3aefa2d2e0286ff0

[emaxx, 2017-03-03 04:43:35]

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

[commit-bot: I haz the power, 2017-03-03 04:43:50]

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

[emaxx, 2017-03-03 04:54:15]

Description was changed from

==========
Fix crash during sync load of the signin profile

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

to

==========
Fix crash during sync load of the signin profile

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier when restore-after-crash happens - before the schema
registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
==========

[emaxx, 2017-03-03 04:55:09]

Description was changed from

==========
Fix crash during sync load of the signin profile

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier when restore-after-crash happens - before the schema
registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
==========

to

==========
Fix crash during sync load of the signin profile

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier when restore-after-crash happens - before the schema
registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

[emaxx, 2017-03-03 05:01:38]

emaxx@chromium.org changed reviewers:
+ tnagel@chromium.org

[emaxx, 2017-03-03 05:01:38]

Thiemo, PTAL ASAP!
This bug affects all enrolled device that run ToT or a recent M58 build.

I'm trying to resolve this issue quickly instead of reverting the culprit CL,
because that one was itself an important P0 fix.

No automated tests are provided currently, mainly for time constraints. I'm
planning to work on the test once this lands.

[emaxx, 2017-03-03 05:10:03]

Description was changed from

==========
Fix crash during sync load of the signin profile

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier when restore-after-crash happens - before the schema
registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

to

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier that previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

[commit-bot: I haz the power, 2017-03-03 05:27:28]

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

[commit-bot: I haz the power, 2017-03-03 05:27:29]

Dry run: This issue passed the CQ dry run.

[emaxx, 2017-03-03 15:41:15]

emaxx@chromium.org changed reviewers:
+ ljusten@chromium.org

[emaxx, 2017-03-03 15:41:15]

Lutz, could you PTAL? Thiemo is OOO.

[emaxx, 2017-03-03 15:46:52]

Description was changed from

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier that previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

to

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

[emaxx, 2017-03-03 15:47:45]

Description was changed from

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

to

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device:
[FATAL:device_cloud_policy_manager_chromeos.cc(252)] Check
failed: signin_profile_forwarding_schema_registry_.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

[emaxx, 2017-03-03 15:54:15]

emaxx@chromium.org changed reviewers:
- ljusten@chromium.org

[emaxx, 2017-03-03 16:20:23]

emaxx@chromium.org changed reviewers:
+ atwilson@chromium.org

[emaxx, 2017-03-03 16:20:24]

Drew, could you PTAL once you return from OOO?

(Removed Lutz as he said he's not familiar enough with the code.)

[emaxx, 2017-03-03 16:21:40]

Description was changed from

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device:
[FATAL:device_cloud_policy_manager_chromeos.cc(252)] Check
failed: signin_profile_forwarding_schema_registry_.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen and verify that
    automatic restart works
==========

to

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device:
[FATAL:device_cloud_policy_manager_chromeos.cc(252)] Check
failed: signin_profile_forwarding_schema_registry_.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen on an enrolled
    device and verify that automatic restart works
==========

[emaxx, 2017-03-03 17:24:36]

emaxx@chromium.org changed reviewers:
+ bartfab@chromium.org

[emaxx, 2017-03-03 17:24:36]

+Bartosz in case you will be able to review it soon (as discussed offline)

[bartfab (slow), 2017-03-03 17:31:28]

lgtm

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
File chrome/browser/policy/profile_policy_connector_factory.cc (right):

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
chrome/browser/policy/profile_policy_connector_factory.cc:12: #include
"chrome/browser/browser_process.h"
Nit: No longer used.

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
chrome/browser/policy/profile_policy_connector_factory.cc:24: #include
"chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
Nit: No longer used.

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
chrome/browser/policy/profile_policy_connector_factory.cc:25: #include
"chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h"
Nit: No longer used.

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/schem...
File chrome/browser/policy/schema_registry_service_factory.cc (right):

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/schem...
chrome/browser/policy/schema_registry_service_factory.cc:131:
policy::DeviceCloudPolicyManagerChromeOS* device_cloud_policy_manager =
Nit: #include
"chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h

[emaxx, 2017-03-03 17:38:46]

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

[commit-bot: I haz the power, 2017-03-03 17:39:16]

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

[emaxx, 2017-03-03 17:42:09]

Thanks for reviewing this quickly!
And for pointing out to the missing/extra includes - as usual, you're extremely
good in spotting them :)

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
File chrome/browser/policy/profile_policy_connector_factory.cc (right):

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
chrome/browser/policy/profile_policy_connector_factory.cc:12: #include
"chrome/browser/browser_process.h"
On 2017/03/03 17:31:28, bartfab (slow) wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
chrome/browser/policy/profile_policy_connector_factory.cc:24: #include
"chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
On 2017/03/03 17:31:28, bartfab (slow) wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/profi...
chrome/browser/policy/profile_policy_connector_factory.cc:25: #include
"chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h"
On 2017/03/03 17:31:28, bartfab (slow) wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/schem...
File chrome/browser/policy/schema_registry_service_factory.cc (right):

https://codereview.chromium.org/2729093002/diff/1/chrome/browser/policy/schem...
chrome/browser/policy/schema_registry_service_factory.cc:131:
policy::DeviceCloudPolicyManagerChromeOS* device_cloud_policy_manager =
On 2017/03/03 17:31:28, bartfab (slow) wrote:
> Nit: #include
> "chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h

Done.

[emaxx, 2017-03-03 17:46:22]

The CQ bit was unchecked by emaxx@chromium.org

[emaxx, 2017-03-03 17:46:29]

The CQ bit was checked by emaxx@chromium.org

[emaxx, 2017-03-03 17:46:30]

The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org
Link to the patchset: https://codereview.chromium.org/2729093002/#ps20001
(title: "Fix nits on includes")

[commit-bot: I haz the power, 2017-03-03 17:47:01]

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, 2017-03-03 18:24:39]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488563189984570,
"parent_rev": "abbd7542f02a4646bdf8c3907d154ef737f78be2", "commit_rev":
"9bae7d67f2a11bc706b9becb3aefa2d2e0286ff0"}

[commit-bot: I haz the power, 2017-03-03 18:25:20]

Description was changed from

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device:
[FATAL:device_cloud_policy_manager_chromeos.cc(252)] Check
failed: signin_profile_forwarding_schema_registry_.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen on an enrolled
    device and verify that automatic restart works
==========

to

==========
Fix crash during sync load of the signin profile

This fixes hitting a CHECK during restore-after-crash of the
signin profile in case of enrolled device:
[FATAL:device_cloud_policy_manager_chromeos.cc(252)] Check
failed: signin_profile_forwarding_schema_registry_.

The crash was caused by an attempt to start connection for
the device cloud policy manager _before_ the schema
registry of the sign-in profile is passed to it.

This has regressed recently due to the change in commit
afc9d60e96d31a11f756d5c56c2be54a28130960, which, in
particular, made the DeviceSettingsService load much
earlier than previously in case when restore-after-crash
happens. The DeviceSettingsService loading triggers
DeviceCloudPolicyManagerChromeOS::StartConnection(),
which hits a CHECK because this all happens before the
schema registry gets a chance to be passed by
ProfilePolicyConnectorFactory::CreateForBrowserContextInternal().

The fix is to pass the schema registry from a different
location - directly from
SchemaRegistryServiceFactory::CreateForContextInternal() that
constructs the schema registry. This is guaranteed by the
current code to be run before the DeviceSettingsService load
happens.

BUG=698136
TEST=manual: induce a crash on signin screen on an enrolled
    device and verify that automatic restart works

Review-Url: https://codereview.chromium.org/2729093002
Cr-Commit-Position: refs/heads/master@{#454627}
Committed:
https://chromium.googlesource.com/chromium/src/+/9bae7d67f2a11bc706b9becb3aef...
==========

[commit-bot: I haz the power, 2017-03-03 18:25:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/9bae7d67f2a11bc706b9becb3aef...