arc: Fix Android App resurrected by sync.

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.h"
 
+#include "chrome/browser/chromeos/arc/arc_auth_service.h"
 #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/arc/arc_bridge_service.h"
 #include "components/prefs/pref_service.h"
+#include "components/sync/driver/pref_names.h"
 #include "components/sync/driver/sync_client.h"
 #include "components/sync/driver/sync_prefs.h"
 #include "components/sync/driver/sync_service.h"
 
 // ArcPackage sync service is controlled by apps checkbox in sync settings. Arc
 // apps and regular Chrome apps have same user control.
+namespace {
+
+bool ValidateEnableArcPackageSyncPref(Profile* profile) {
+  PrefService* pref_service = profile->GetPrefs();

[pavely, 2016/09/28 20:19:01]

You can use SyncService::GetPreferredDataTypes() and check if ARC_PACKAGE is in
returned ModelTypeSet. It will cover checking "Sync everything" as well as
including ARC_PACKAGE when APPS checkbox is enabled.

[lgcheng, 2016/09/28 21:40:36]

Done.

+  // If device is set to sync everything, Arc package should be synced.
+  if (pref_service->GetBoolean(sync_driver::prefs::kSyncKeepEverythingSynced))
+    return true;
+
+  const bool apps_sync_enable = pref_service->GetBoolean(
+      sync_driver::SyncPrefs::GetPrefNameForDataType(syncer::APPS));
+  // ArcPackage sync service is controlled by apps checkbox in sync settings.
+  // Update ArcPackage sync setting pref if it is different from apps sync
+  // setting pref.
+  const char* arc_sync_path =
+      sync_driver::SyncPrefs::GetPrefNameForDataType(syncer::ARC_PACKAGE);
+  if (apps_sync_enable != pref_service->GetBoolean(arc_sync_path)) {
+    pref_service->SetBoolean(arc_sync_path, apps_sync_enable);

[pavely, 2016/09/28 20:19:01]

Why do you need to implement this logic? Is there a scenario you ran into that
you want to cover?

[lgcheng, 2016/09/28 21:40:36]

There was a scenario that user chose what to sync(not sync everything) before
updating the patch which associates ARC_PACKAGE to APPS. Then after update with
the patch ARC_PACKAGE sync pref is still unchecked while APPS was checked before
the patch and ARC_PACKAGE sync service will not start. But it would not
reproduce if I use SyncService::GetPreferredDataTypes() and check if ARC_PACKAGE
is inreturned ModelTypeSet.

+  }
+
+  return apps_sync_enable;
+}
+
+// Indicates whether ARC is enabled on this machine.
+bool IsArcEnabled(Profile* profile) {
+  return arc::ArcAuthService::IsAllowedForProfile(profile) &&
+      profile->GetPrefs()->GetBoolean(prefs::kArcEnabled);
+}
+
+}  // namespace
+
 ArcPackageSyncDataTypeController::ArcPackageSyncDataTypeController(
     syncer::ModelType type,
     const base::Closure& dump_stack,
     sync_driver::SyncClient* sync_client,
     Profile* profile)
     : sync_driver::UIDataTypeController(type, dump_stack, sync_client),
       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)));
+
+  arc::ArcBridgeService* arc_bridge_service = arc::ArcBridgeService::Get();
+  if (arc_bridge_service)
+    arc_bridge_service->app()->AddObserver(this);
 }
 
-ArcPackageSyncDataTypeController::~ArcPackageSyncDataTypeController() {}
+ArcPackageSyncDataTypeController::~ArcPackageSyncDataTypeController() {
+  arc::ArcBridgeService* arc_bridge_service = arc::ArcBridgeService::Get();
+  if (arc_bridge_service)
+    arc_bridge_service->app()->RemoveObserver(this);
+}
 
 bool ArcPackageSyncDataTypeController::ReadyForStart() const {
   DCHECK(CalledOnValidThread());
+  return IsArcEnabled(profile_) && ValidateEnableArcPackageSyncPref(profile_);
+}
+
+void ArcPackageSyncDataTypeController::OnInstanceReady() {
+  // model_normal_start_ is true by default. Normally,
+  // ArcPackageSyncDataTypeController::StartModels() gets called before Arc is
+  // ready. But in integration test, the order can be either way. If
+  // OnInstanceReady comes before ArcPackageSyncDataTypeController
+  // ::StartModels(), this function is no-op and waits for StartModels() to be
+  // called.
+  if (model_normal_start_)
+    return;
+
+  model_normal_start_ = true;
+  OnModelLoaded();
+}
+
+bool ArcPackageSyncDataTypeController::StartModels() {
+  DCHECK_EQ(state(), MODEL_STARTING);
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_);
-  return profile_->GetPrefs()->GetBoolean(
-             sync_driver::SyncPrefs::GetPrefNameForDataType(type())) &&
-         prefs && prefs->app_instance_holder()->has_instance();
+  model_normal_start_ = prefs && prefs->app_instance_holder()->has_instance();
+  return model_normal_start_;
 }
 
 void ArcPackageSyncDataTypeController::OnArcAppsSyncPrefChanged() {
   DCHECK(CalledOnValidThread());
 
   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());
       CreateErrorHandler()->OnUnrecoverableError(error);
     }
     return;
   }
-  sync_driver::SyncService* sync_service = sync_client_->GetSyncService();
-  DCHECK(sync_service);
-  sync_service->ReenableDatatype(type());
+  EnableDataType();
 }
 
 void ArcPackageSyncDataTypeController::OnArcEnabledPrefChanged() {
   DCHECK(CalledOnValidThread());
-  if (!profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
+
+  if (!ReadyForStart()) {
     // 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());
       CreateErrorHandler()->OnUnrecoverableError(error);
     }
+    return;
   }
+  EnableDataType();
 }
+
+void ArcPackageSyncDataTypeController::EnableDataType() {
+  sync_driver::SyncService* sync_service = sync_client_->GetSyncService();
+  DCHECK(sync_service);
+  sync_service->ReenableDatatype(type());
+}