arc: Enable user control for Arc package sync.

Current Arc package sync implemetation does not support stop sync and re-enable
sync without rebooting. So user control of arc package sync is not achievable.
This patch re-implement sync stop and enable proper user control of arc package
sync using apps checkbox in advanced sync settings.

BUG= http://b/31034323, http://b/30640291, http://b/30980543
TEST=Pass sync integration test.
TEST=Manual test1. Turn on apps sync settings. Enable Arc and install package.
Nuke Arc then enable arc. Package restored.
TEST=Manual test2. Turn on apps sync settings. Enable Arc and install package.
Turn off apps sync settings and then nuke Arc. Then enable arc. Package not
restored. Then turn on apps sync settings. Package restored.

Review-Url: https://codereview.chromium.org/2277593002
Cr-Commit-Position: refs/heads/master@{#414277}

chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc

@@ -585,9 +585,6 @@ void ArcAppListPrefs::OnInstanceReady() {
 
   app_instance->Init(binding_.CreateInterfacePtrAndBind());
   app_instance->RefreshAppList();
-
-  // Start ArcPackageSyncService.
-  sync_service_->SyncStarted();
 }
 
 void ArcAppListPrefs::OnInstanceClosed() {
@@ -989,6 +986,13 @@ void ArcAppListPrefs::OnPackageListRefreshed(
     if (!current_packages.count(package_name))
       RemovePackageFromPrefs(prefs_, package_name);
   }
+
+  // Start ArcPackageSyncService ASAP. This is the call_back of
+  // app_instance->RefreshAppList() in OnInstanceReady(). SyncStarted() should
+  // only be called after packagelist refresh is completed. SyncStarted() is
+  // no-op after first time setup or if sync is disabled.
+  DCHECK(sync_service_);
+  sync_service_->SyncStarted();
 }
 
 std::vector<std::string> ArcAppListPrefs::GetPackagesFromPrefs() const {

chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc

@@ -6,10 +6,17 @@
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
+#include "chrome/common/pref_names.h"
 #include "components/arc/common/app.mojom.h"
 #include "components/arc/instance_holder.h"
+#include "components/prefs/pref_service.h"
+#include "components/sync/driver/sync_client.h"
+#include "components/sync/driver/sync_prefs.h"
+#include "components/sync/driver/sync_service.h"
 #include "content/public/browser/browser_thread.h"
 
+// ArcPackage sync service is controlled by apps checkbox in sync settings. Arc
+// apps and regular Chrome apps have same user control.
 ArcPackageSyncDataTypeController::ArcPackageSyncDataTypeController(
     syncer::ModelType type,
     const base::Closure& error_callback,
@@ -21,13 +28,57 @@ ArcPackageSyncDataTypeController::ArcPackageSyncDataTypeController(
           error_callback,
           type,
           sync_client),
-      profile_(profile) {}
+      profile_(profile),
+      sync_client_(sync_client) {
+  pref_registrar_.Init(profile_->GetPrefs());
+  pref_registrar_.Add(
+      sync_driver::SyncPrefs::GetPrefNameForDataType(type),
+      base::Bind(&ArcPackageSyncDataTypeController::OnArcAppsSyncPrefChanged,
+                 base::Unretained(this)));
+  pref_registrar_.Add(
+      prefs::kArcEnabled,
+      base::Bind(&ArcPackageSyncDataTypeController::OnArcEnabledPrefChanged,
+                 base::Unretained(this)));
+}
 
 ArcPackageSyncDataTypeController::~ArcPackageSyncDataTypeController() {}
 
 bool ArcPackageSyncDataTypeController::ReadyForStart() const {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_);
-  if (prefs && prefs->app_instance_holder()->instance())
-    return true;
-  return false;
+  return profile_->GetPrefs()->GetBoolean(
+             sync_driver::SyncPrefs::GetPrefNameForDataType(type())) &&
+         prefs && prefs->app_instance_holder()->instance();
+}
+
+void ArcPackageSyncDataTypeController::OnArcAppsSyncPrefChanged() {
+  DCHECK(ui_thread()->BelongsToCurrentThread());
+
+  if (!ReadyForStart()) {
+    // If apps sync in advanced sync settings is turned off then generate an
+    // unrecoverable error.
+    if (state() != NOT_RUNNING && state() != STOPPING) {
+      syncer::SyncError error(
+          FROM_HERE, syncer::SyncError::DATATYPE_POLICY_ERROR,
+          "Arc package sync is now disabled by user.", type());
+      OnSingleDataTypeUnrecoverableError(error);
+    }
+    return;
+  }
+  sync_driver::SyncService* sync_service = sync_client_->GetSyncService();
+  DCHECK(sync_service);
+  sync_service->ReenableDatatype(type());
+}
+
+void ArcPackageSyncDataTypeController::OnArcEnabledPrefChanged() {
+  if (!profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
+    // If enable Arc in settings is turned off then generate an unrecoverable
+    // error.
+    if (state() != NOT_RUNNING && state() != STOPPING) {
+      syncer::SyncError error(
+          FROM_HERE, syncer::SyncError::DATATYPE_POLICY_ERROR,
+          "Arc package sync is now disabled because user disables Arc.",
+          type());
+      OnSingleDataTypeUnrecoverableError(error);
+    }
+  }
 }

chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_APP_LIST_ARC_ARC_PACKAGE_SYNC_DATA_TYPE_CONTROLLER_H_
 
 #include "base/macros.h"
+#include "components/prefs/pref_change_registrar.h"
 #include "components/sync/driver/data_type_controller.h"
 #include "components/sync/driver/ui_data_type_controller.h"
 
@@ -31,8 +32,16 @@ class ArcPackageSyncDataTypeController
   // DataTypeController is RefCounted.
   ~ArcPackageSyncDataTypeController() override;
 
+  void OnArcAppsSyncPrefChanged();
+
+  void OnArcEnabledPrefChanged();
+
   Profile* const profile_;
 
+  sync_driver::SyncClient* sync_client_;
+
+  PrefChangeRegistrar pref_registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(ArcPackageSyncDataTypeController);
 };
 #endif  // CHROME_BROWSER_UI_APP_LIST_ARC_ARC_PACKAGE_SYNC_DATA_TYPE_CONTROLLER_H_

chrome/browser/ui/app_list/arc/arc_package_syncable_service.cc

@@ -15,6 +15,7 @@
 #include "components/sync/api/sync_change_processor.h"
 #include "components/sync/api/sync_data.h"
 #include "components/sync/api/sync_merge_result.h"
+#include "components/sync/driver/sync_prefs.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/protocol/sync.pb.h"
 
@@ -229,8 +230,14 @@ bool ArcPackageSyncableService::SyncStarted() {
 
   sync_driver::SyncService* sync_service =
       ProfileSyncServiceFactory::GetSyncServiceForBrowserContext(profile_);
-  if (sync_service)
-    sync_service->ReenableDatatype(syncer::ARC_PACKAGE);
+  // ArcPackage sync service is controlled by apps checkbox in sync settings.
+  bool arc_package_sync_should_enable = profile_->GetPrefs()->GetBoolean(
+      sync_driver::SyncPrefs::GetPrefNameForDataType(syncer::ARC_PACKAGE));
+
+  if (!sync_service || !arc_package_sync_should_enable)
+    return false;
+
+  sync_service->ReenableDatatype(syncer::ARC_PACKAGE);
 
   if (flare_.is_null()) {
     VLOG(2) << this << ": SyncStarted: Flare.";

components/sync/driver/sync_prefs.cc

@@ -354,6 +354,7 @@ void SyncPrefs::RegisterPrefGroups() {
   pref_groups_[syncer::APPS].Put(syncer::APP_NOTIFICATIONS);
   pref_groups_[syncer::APPS].Put(syncer::APP_SETTINGS);
   pref_groups_[syncer::APPS].Put(syncer::APP_LIST);
+  pref_groups_[syncer::APPS].Put(syncer::ARC_PACKAGE);
 
   pref_groups_[syncer::AUTOFILL].Put(syncer::AUTOFILL_PROFILE);
   pref_groups_[syncer::AUTOFILL].Put(syncer::AUTOFILL_WALLET_DATA);

components/sync/driver/sync_prefs_unittest.cc

@@ -153,6 +153,7 @@ TEST_F(SyncPrefsTest, PreferredTypesNotKeepEverythingSynced) {
       expected_preferred_types.Put(syncer::APP_LIST);
       expected_preferred_types.Put(syncer::APP_NOTIFICATIONS);
       expected_preferred_types.Put(syncer::APP_SETTINGS);
+      expected_preferred_types.Put(syncer::ARC_PACKAGE);
     }
     if (it.Get() == syncer::EXTENSIONS) {
       expected_preferred_types.Put(syncer::EXTENSION_SETTINGS);