Migrate ArcUserDataService into ArcAuthService.

chrome/browser/chromeos/arc/arc_auth_service.cc

 // Copyright 2015 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/chromeos/arc/arc_auth_service.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 216 matching lines @@
 }
 
 void ArcAuthService::OnBridgeStopped(ArcBridgeService::StopReason reason) {
   // TODO(crbug.com/625923): Use |reason| to report more detailed errors.
   if (waiting_for_reply_) {
     // Using SERVICE_UNAVAILABLE instead of UNKNOWN_ERROR, since the latter
     // causes this code to not try to stop ARC, so it would retry without the
     // 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() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   std::string auth_code;
   auth_code_.swap(auth_code);
   return auth_code;
 }
 
 void ArcAuthService::GetAuthCodeDeprecated(
@@ skipping 91 matching lines @@
       profile_->GetPrefs()->SetBoolean(prefs::kArcSignedIn, false);
     ShutdownBridgeAndShowUI(UIPage::ERROR,
                             l10n_util::GetStringUTF16(error_message_id));
     return;
   }
 
   if (result == ProvisioningResult::CLOUD_PROVISION_FLOW_FAILED ||
       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.
   ShowUI(UIPage::ERROR_WITH_FEEDBACK,
          l10n_util::GetStringUTF16(error_message_id));
 }
 
 void ArcAuthService::GetIsAccountManaged(
     const GetIsAccountManagedCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ skipping 151 matching lines @@
 
 void ArcAuthService::OnOptInPreferenceChanged() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
 
   const bool arc_enabled = IsArcEnabled();
   FOR_EACH_OBSERVER(Observer, observer_list_, OnOptInEnabled(arc_enabled));
 
   if (!arc_enabled) {
     StopArc();
+    RemoveArcData();
     return;
   }
 
   if (state_ == State::ACTIVE)
     return;
   CloseUI();
   auth_code_.clear();
 
   if (!profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn)) {
     // Need pre-fetch auth code and show OptIn UI if needed.
@@ skipping 50 matching lines @@
     ArcAuthNotification::Hide();
 }
 
 void ArcAuthService::SetUIPage(UIPage page, const base::string16& status) {
   ui_page_ = page;
   ui_page_status_ = status;
   FOR_EACH_OBSERVER(Observer, observer_list_,
                     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();
   SetState(State::ACTIVE);
 }
 
 void ArcAuthService::SetAuthCodeAndStartArc(const std::string& auth_code) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!auth_code.empty());
 
@@ skipping 199 matching lines @@
       return os << kStateFetchingCode;
     case ArcAuthService::State::ACTIVE:
       return os << kStateActive;
     default:
       NOTREACHED();
       return os;
   }
 }
 
 }  // namespace arc