Fix DisableReason handling in ProfileSyncService::Initialize

ProfileSyncService::Initialize used to early-out if
DISABLE_REASON_PLATFORM_OVERRIDE or DISABLE_REASON_ENTERPRISE_POLICY
were present. However:
- In practice, platform_sync_allowed_provider_ isn't set at this point,
  so DISABLE_REASON_PLATFORM_OVERRIDE will never occur. Even if it did,
  calling StopImpl(KEEP_DATA) at this point (before |engine_| exists)
  does nothing.
- Both disable reasons can disappear during Chrome's lifetime, in which
  case ProfileSyncService is left in a half-initialized state. In
  particular, it won't be listening for auth events.

This CL removes handling for DISABLE_REASON_PLATFORM_OVERRIDE, and does
not early-out anymore for DISABLE_REASON_ENTERPRISE_POLICY.

Bug: 870683
Change-Id: I354629deaa42a782e10658190c84e967fee593a3
Reviewed-on: https://chromium-review.googlesource.com/1162236Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580818}

components/browser_sync/profile_sync_service.cc

@@ -303,13 +303,11 @@ void ProfileSyncService::Initialize() {
   int disable_reasons = GetDisableReasons();
   RecordSyncInitialState(disable_reasons, IsFirstSetupComplete());
 
-  // If sync isn't allowed, the only thing to do is to turn it off.
-  if ((disable_reasons & DISABLE_REASON_PLATFORM_OVERRIDE) ||
-      (disable_reasons & DISABLE_REASON_ENTERPRISE_POLICY)) {
-    // Only clear data if disallowed by policy.
-    StopImpl((disable_reasons & DISABLE_REASON_ENTERPRISE_POLICY) ? CLEAR_DATA
-                                                                  : KEEP_DATA);
-    return;
+  // If sync is disallowed by policy, clean up.
+  if (disable_reasons & DISABLE_REASON_ENTERPRISE_POLICY) {
+    // Note that this won't actually clear data, since neither |engine_| nor
+    // |sync_thread_| exist at this point. Bug or feature?
+    StopImpl(CLEAR_DATA);
   }
 
   if (!IsLocalSyncEnabled()) {

components/browser_sync/profile_sync_service_unittest.cc

@@ -479,6 +479,33 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyBeforeInit) {
   EXPECT_EQ(syncer::SyncService::State::DISABLED, service()->GetState());
 }
 
+// This test exercises sign-in after startup, which isn't supported on ChromeOS.
+#if !defined(OS_CHROMEOS)
+TEST_F(ProfileSyncServiceTest, DisabledByPolicyBeforeInitThenPolicyRemoved) {
+  prefs()->SetManagedPref(syncer::prefs::kSyncManaged,
+                          std::make_unique<base::Value>(true));
+  CreateService(ProfileSyncService::AUTO_START);
+  InitializeForNthSync();
+  EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY |
+                syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN,
+            service()->GetDisableReasons());
+  EXPECT_EQ(syncer::SyncService::State::DISABLED, service()->GetState());
+
+  // Remove the policy. Now only missing sign-in is preventing startup.
+  prefs()->SetManagedPref(syncer::prefs::kSyncManaged,
+                          std::make_unique<base::Value>(false));
+  EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN,
+            service()->GetDisableReasons());
+  EXPECT_EQ(syncer::SyncService::State::DISABLED, service()->GetState());
+
+  // Once we mark first setup complete again (it was cleared by the policy) and
+  // sign in, sync starts up.
+  service()->SetFirstSetupComplete();
+  SignIn();
+  EXPECT_EQ(syncer::SyncService::State::ACTIVE, service()->GetState());
+}
+#endif  // !defined(OS_CHROMEOS)
+
 // Verify that disable by enterprise policy works even after the backend has
 // been initialized.
 TEST_F(ProfileSyncServiceTest, DisabledByPolicyAfterInit) {