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 {
+
+// 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(

[pavely, 2016/09/29 08:49:02]

You don't need to register for notifications on sync pref changes. There is no
need to generate unrecoverable error when the pref turns to false or to
explicitly reenable type when pref changes to true. Sync engine monitors these
changes and will start/stop datatypes as needed. 

Basically I'm proposing to remove observing
SyncPrefs::GetPrefNameForDataType(type) pref and OnArcAppsSyncPrefChanged
function.

You still need to monitor prefs::kArcEnabled though.

Feel free to make this change in separate CL if it is more convenient.

[lgcheng, 2016/09/29 17:52:48]

I think this is very neat suggestion to remove the redundancy. As I am going to
merge the patch to M54, I'd like to remove unnecessary observer in this patch.

       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_) && ShouldSyncArc();
+}
+
+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());
+}
+
+bool ArcPackageSyncDataTypeController::ShouldSyncArc() const {
+  sync_driver::SyncService* sync_service = sync_client_->GetSyncService();
+  DCHECK(sync_service);
+  return sync_service->GetPreferredDataTypes().Has(type());
+}