arc: Make request to remove Android's data folder persistent.

chrome/browser/chromeos/arc/arc_session_manager.cc

Index: chrome/browser/chromeos/arc/arc_session_manager.cc
diff --git a/chrome/browser/chromeos/arc/arc_session_manager.cc b/chrome/browser/chromeos/arc/arc_session_manager.cc
index 31595541115c22fa6f4ec32dd325f565805f6fdf..445ed0d75860429fecb3c026eea703d07ce123ed 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager.cc
@@ -110,6 +110,7 @@ void ArcSessionManager::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   // TODO(dspaid): Implement a mechanism to allow this to sync on first boot
   // only.
+  registry->RegisterBooleanPref(prefs::kArcDataRemoveRequested, false);
   registry->RegisterBooleanPref(prefs::kArcEnabled, false);
   registry->RegisterBooleanPref(prefs::kArcSignedIn, false);
   registry->RegisterBooleanPref(prefs::kArcTermsAccepted, false);
@@ -195,30 +196,33 @@ void ArcSessionManager::OnBridgeStopped(ArcBridgeService::StopReason reason) {
     OnProvisioningFinished(ProvisioningResult::ARC_STOPPED);
   }
 
-  if (clear_required_) {
+  if (profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested)) {
     // This should be always true, but just in case as this is looked at
     // inside RemoveArcData() at first.
     DCHECK(arc_bridge_service()->stopped());
     RemoveArcData();
   } else {
     // To support special "Stop and enable ARC" procedure for enterprise,
-    // here call OnArcDataRemoved(true) as if the data removal is successfully
-    // done.
-    // TODO(hidehiko): Restructure the code. crbug.com/665316
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&ArcSessionManager::OnArcDataRemoved,
-                              weak_ptr_factory_.GetWeakPtr(), true));
+    // here call MaybeReenableArc()
+    MaybeReenableArc();

[hidehiko, 2016/12/06 05:05:30]

PostTask is the workaround for crbug.com/665316, and it is not yet fixed, so I
think it still needs to be kept here.

[khmel, 2016/12/06 18:16:13]

It seems that problem quite complicated based on task desc. Let address in
separate CL.

   }
 }
 
 void ArcSessionManager::RemoveArcData() {
+  // Ignore redundant data removal request.
+  if (state() == State::DELETING_DATA_FOLDER)
+    return;
+
+  // OnArcDataRemoved resets this flag.
+  profile_->GetPrefs()->SetBoolean(prefs::kArcDataRemoveRequested, true);
+
   if (!arc_bridge_service()->stopped()) {
     // Just set a flag. On bridge stopped, this will be re-called,
     // then session manager should remove the data.
-    clear_required_ = true;
     return;
   }
-  clear_required_ = false;
+
+  SetState(State::DELETING_DATA_FOLDER);
   chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->RemoveArcData(
       cryptohome::Identification(
           multi_user_util::GetAccountIdFromProfile(profile_)),
@@ -229,12 +233,24 @@ void ArcSessionManager::RemoveArcData() {
 void ArcSessionManager::OnArcDataRemoved(bool success) {
   LOG_IF(ERROR, !success) << "Required ARC user data wipe failed.";
 
+  // TODO(khmel): Browser tests may shutdown profile by itself. Update browser
+  // tests and remove this check.
+  if (state() == State::NOT_INITIALIZED)
+    return;
+
+  profile_->GetPrefs()->SetBoolean(prefs::kArcDataRemoveRequested, false);
+  DCHECK_EQ(state(), State::DELETING_DATA_FOLDER);
+  SetState(State::STOPPED);
+
+  MaybeReenableArc();
+}
+
+void ArcSessionManager::MaybeReenableArc() {
   // Here check if |reenable_arc_| is marked or not.
   // The only case this happens should be in the special case for enterprise
   // "on managed lost" case. In that case, OnBridgeStopped() should trigger
   // the RemoveArcData(), then this.
-  // TODO(hidehiko): Restructure the code.
-  if (!reenable_arc_)
+  if (!reenable_arc_ || !IsArcEnabled())
     return;
 
   // Restart ARC anyway. Let the enterprise reporting instance decide whether
@@ -276,6 +292,10 @@ void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
     if (support_host_)
       support_host_->Close();
 
+    // No data removal request is expected on Arc boot. Removing Arc data is
+    // always accompanied with request to stop the bridge.
+    DCHECK(!profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested));
+
     if (profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn))
       return;
 
@@ -412,7 +432,14 @@ void ArcSessionManager::OnPrimaryUserProfilePrepared(Profile* profile) {
       base::Bind(&ArcSessionManager::OnOptInPreferenceChanged,
                  weak_ptr_factory_.GetWeakPtr()));
   if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
-    OnOptInPreferenceChanged();
+    // Don't start ARC if there is a pending request to remove the data. Restart
+    // ARC once data removal finishes.
+    if (profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested)) {
+      reenable_arc_ = true;
+      RemoveArcData();
+    } else {
+      OnOptInPreferenceChanged();
+    }
   } else {
     RemoveArcData();
     PrefServiceSyncableFromProfile(profile_)->AddObserver(this);
@@ -492,6 +519,8 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
     observer.OnOptInEnabled(arc_enabled);
 
   if (!arc_enabled) {
+    // Reset any pending request to re-enable Arc.
+    reenable_arc_ = false;
     StopArc();
     RemoveArcData();
     return;
@@ -500,6 +529,13 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
   if (state_ == State::ACTIVE)
     return;
 
+  if (state_ == State::DELETING_DATA_FOLDER) {
+    // Data removal request is in progress. Set flag to re-enable Arc once it is
+    // finished.
+    reenable_arc_ = true;
+    return;
+  }
+
   if (support_host_)
     support_host_->SetArcManaged(IsArcManaged());
 
@@ -564,7 +600,7 @@ void ArcSessionManager::ShutdownBridge() {
   terms_of_service_negotiator_.reset();
   android_management_checker_.reset();
   arc_bridge_service()->RequestStop();
-  if (state_ != State::NOT_INITIALIZED)
+  if (state_ != State::NOT_INITIALIZED && state_ != State::DELETING_DATA_FOLDER)
     SetState(State::STOPPED);
   for (auto& observer : observer_list_)
     observer.OnShutdownBridge();
@@ -591,6 +627,10 @@ void ArcSessionManager::StopAndEnableArc() {
 
 void ArcSessionManager::StartArc() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Arc must be started only if no pending data removal request exists.
+  DCHECK(!profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested));
+
   arc_bridge_service()->RequestStart();
   SetState(State::ACTIVE);
 }
@@ -849,6 +889,8 @@ std::ostream& operator<<(std::ostream& os,
       return os << "SHOWING_TERMS_OF_SERVICE";
     case ArcSessionManager::State::CHECKING_ANDROID_MANAGEMENT:
       return os << "CHECKING_ANDROID_MANAGEMENT";
+    case ArcSessionManager::State::DELETING_DATA_FOLDER:
+      return os << "DELETING_DATA_FOLDER";
     case ArcSessionManager::State::ACTIVE:
       return os << "ACTIVE";
   }