Migrate ArcUserDataService into ArcAuthService.

chrome/browser/chromeos/arc/arc_auth_service.cc

Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc b/chrome/browser/chromeos/arc/arc_auth_service.cc
index 3aaffd8a7f8ea675eb0469d64de27c42d2199212..fd7cc3b1f944c637c1502cae16d1deb00e77642d 100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -234,13 +234,41 @@ void ArcAuthService::OnBridgeStopped(ArcBridgeService::StopReason reason) {
     // user noticing.
     OnSignInFailedInternal(ProvisioningResult::ARC_STOPPED);
   }
-  if (!clear_required_)
+
+  if (clear_required_) {
+    clear_required_ = false;
+    RemoveArcData();
+  } else {
+    OnArcDataRemoved(true);

[khmel, 2016/08/04 23:23:29]

nit: this used quite confusion for me. May we rename it or split into 2
functions:
OnArcRemoved()
MayReenableArc()?

[hidehiko, 2016/08/09 13:23:56]

I agree that this may be confusing, but TBH, your splitting suggestion also
confusing to me, because calling MayReenableArc() in both here and
OnArcDataRemoved() does not have any good reasoning.

So, instead I added comments. Does it work for you?
Note that I'd like to drastically restructure the code, but the considering the
size and its risk, I'll do it in later release.

[khmel, 2016/08/09 15:23:11]

sure, it was only nit :)

+  }
+}
+
+void ArcAuthService::RemoveArcData() {
+  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;
   chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->RemoveArcData(
       cryptohome::Identification(
           multi_user_util::GetAccountIdFromProfile(profile_)),
-      chromeos::SessionManagerClient::ArcCallback());
+      base::Bind(&ArcAuthService::OnArcDataRemoved,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ArcAuthService::OnArcDataRemoved(bool success) {
+  LOG_IF(ERROR, !success) << "Required ARC user data wipe failed.";
+
+  if (!reenable_arc_)
+    return;
+
+  // Restart ARC anyway. Let the enterprise reporting instance decide whether
+  // the ARC user data wipe is still required or not.
+  reenable_arc_ = false;
+  VLOG(1) << "Reenable ARC";
+  EnableArc();
 }
 
 std::string ArcAuthService::GetAndResetAuthCode() {
@@ -352,7 +380,7 @@ void ArcAuthService::OnSignInFailedInternal(ProvisioningResult result) {
       result == ProvisioningResult::CLOUD_PROVISION_FLOW_TIMEOUT ||
       result == ProvisioningResult::CLOUD_PROVISION_FLOW_INTERNAL_ERROR ||
       result == ProvisioningResult::UNKNOWN_ERROR)
-    clear_required_ = true;
+    RemoveArcData();

[khmel, 2016/08/04 23:23:29]

nit: {}

[hidehiko, 2016/08/09 13:23:56]

Let me keep this as is for consistency. 
No {} for oneline-if-body.

[khmel, 2016/08/09 15:23:11]

We have multi-line conditions. Based on my previous CLs I was always recommended
by reviewers to use {} in such cases.

 
   // We'll delay shutting down the bridge in this case to allow people to send
   // feedback.
@@ -524,6 +552,7 @@ void ArcAuthService::OnOptInPreferenceChanged() {
 
   if (!arc_enabled) {
     StopArc();
+    RemoveArcData();
     return;
   }
 
@@ -594,6 +623,15 @@ void ArcAuthService::SetUIPage(UIPage page, const base::string16& status) {
                     OnOptInUIShowPage(ui_page_, ui_page_status_));
 }
 
+// This is the special method to support enterprise mojo API.
+// TODO(hidehiko): Remove this.
+void ArcAuthService::ReenableArc() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(!arc_bridge_service()->stopped());
+  reenable_arc_ = true;
+  StopArc();
+}
+
 void ArcAuthService::StartArc() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   arc_bridge_service()->HandleStartup();