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..198404f297cec684a9b8a02a452b5b7356243ff9 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,36 @@ 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.
+    // here call MaybeReenableArc() asyncronously.
     // TODO(hidehiko): Restructure the code. crbug.com/665316
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&ArcSessionManager::OnArcDataRemoved,
-                              weak_ptr_factory_.GetWeakPtr(), true));
+        FROM_HERE, base::Bind(&ArcSessionManager::MaybeReenableArc,
+                              weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
 void ArcSessionManager::RemoveArcData() {
+  // Ignore redundant data removal request.
+  if (state() == State::REMOVING_DATA_DIR)
+    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::REMOVING_DATA_DIR);
   chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->RemoveArcData(
       cryptohome::Identification(
           multi_user_util::GetAccountIdFromProfile(profile_)),
@@ -229,12 +236,27 @@ 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;
+
+  for (auto& observer : observer_list_)
+    observer.OnArcDataRemoved();
+
+  profile_->GetPrefs()->SetBoolean(prefs::kArcDataRemoveRequested, false);
+  DCHECK_EQ(state(), State::REMOVING_DATA_DIR);
+  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
@@ -246,7 +268,11 @@ void ArcSessionManager::OnArcDataRemoved(bool success) {
 
 void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK_EQ(state_, State::ACTIVE);
+
+  // Due asynchronous nature of stopping Arc bridge, OnProvisioningFinished may

[hidehiko, 2016/12/06 18:32:10]

Nice commenting!

+  // arrive after setting the |State::STOPPED| state and |State::Active| is not
+  // guaranty set here. prefs::kArcDataRemoveRequested is also can be active
+  // for now.
 
   if (result == ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR) {
     if (IsArcKioskMode()) {
@@ -412,7 +438,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 +525,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 +535,13 @@ void ArcSessionManager::OnOptInPreferenceChanged() {
   if (state_ == State::ACTIVE)
     return;
 
+  if (state_ == State::REMOVING_DATA_DIR) {
+    // 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 +606,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::REMOVING_DATA_DIR)
     SetState(State::STOPPED);
   for (auto& observer : observer_list_)
     observer.OnShutdownBridge();
@@ -591,6 +633,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 +895,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::REMOVING_DATA_DIR:
+      return os << "REMOVING_DATA_DIR";
     case ArcSessionManager::State::ACTIVE:
       return os << "ACTIVE";
   }