Turn ArcSupportHost from Observer model to Delegate

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 f3ab8b7a2911ee590d2191efb52f09589700c5d4..b0302a09ff7c9dd9381fe592dde45292cc587a43 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager.cc
@@ -426,7 +426,7 @@ void ArcSessionManager::SetProfile(Profile* profile) {
       !IsArcKioskMode()) {
     DCHECK(!support_host_);
     support_host_ = base::MakeUnique<ArcSupportHost>(profile_);
-    support_host_->AddObserver(this);
+    support_host_->SetErrorDelegate(this);
   }
 
   DCHECK_EQ(State::NOT_INITIALIZED, state_);
@@ -448,8 +448,8 @@ void ArcSessionManager::Shutdown() {
   enable_requested_ = false;
   ShutdownSession();
   if (support_host_) {
+    support_host_->SetErrorDelegate(nullptr);
     support_host_->Close();
-    support_host_->RemoveObserver(this);
     support_host_.reset();
   }
   context_.reset();
@@ -953,37 +953,18 @@ void ArcSessionManager::MaybeReenableArc() {
   RequestEnableImpl();
 }
 
-void ArcSessionManager::OnWindowClosed() {
-  DCHECK(support_host_);
-  if (terms_of_service_negotiator_) {
-    // In this case, ArcTermsOfServiceNegotiator should handle the case.
-    // Do nothing.
-    return;
-  }
+void ArcSessionManager::OnOptInAborted() {
   CancelAuthCode();
 }
 
-void ArcSessionManager::OnTermsAgreed(bool is_metrics_enabled,
-                                      bool is_backup_and_restore_enabled,
-                                      bool is_location_service_enabled) {
-  DCHECK(support_host_);
-  DCHECK(terms_of_service_negotiator_);
-  // This should be handled in ArcTermsOfServiceNegotiator. Do nothing here.
-}
-
 void ArcSessionManager::OnRetryClicked() {
   DCHECK(support_host_);
+  DCHECK_EQ(support_host_->ui_page(), ArcSupportHost::UIPage::ERROR);
+  DCHECK(!terms_of_service_negotiator_);
 
   UpdateOptInActionUMA(OptInActionType::RETRY);
 
-  // TODO(hidehiko): Simplify the retry logic.
-  if (terms_of_service_negotiator_) {
-    // Currently Terms of service is shown. ArcTermsOfServiceNegotiator should
-    // handle this.
-  } else if (!profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) {
-    MaybeStartTermsOfServiceNegotiation();
-  } else if (support_host_->ui_page() == ArcSupportHost::UIPage::ERROR &&
-             !arc_session_runner_->IsStopped()) {
+  if (!arc_session_runner_->IsStopped()) {
     // ERROR_WITH_FEEDBACK is set in OnSignInFailed(). In the case, stopping

[Luis Héctor Chávez, 2017/05/12 15:41:30]

This comment seems to be out of sync with the code now. Maybe just omit the
ERROR_WITH_FEEDBACK bit.

[victorhsieh0, 2017/05/12 18:31:54]

Done.

     // ARC was postponed to contain its internal state into the report.
     // Here, on retry, stop it, then restart.
@@ -991,9 +972,6 @@ void ArcSessionManager::OnRetryClicked() {
     support_host_->ShowArcLoading();
     ShutdownSession();
     reenable_arc_ = true;
-  } else if (state_ == State::ACTIVE) {
-    // This case is handled in ArcAuthService.
-    // Do nothing.
   } else {
     // Otherwise, we restart ARC. Note: this is the first boot case.
     // For second or later boot, either ERROR_WITH_FEEDBACK case or ACTIVE

[Luis Héctor Chávez, 2017/05/12 15:41:30]

This, too.

[victorhsieh0, 2017/05/12 18:31:55]

Done.  Please let me know if there is something worth to comment about, e.g. if
it should still happen in certain condition.