Show notification during ARC managed provision

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 98cf2ba90edae8804e6707e34880abf5dd068094..80fc1670847955551498a5ca4ee003a4c47be40d 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager.cc
@@ -311,8 +311,7 @@ void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
     if (profile_->GetPrefs()->HasPrefPath(prefs::kArcSignedIn))
       profile_->GetPrefs()->SetBoolean(prefs::kArcSignedIn, false);
     ShutdownSession();
-    if (support_host_)
-      support_host_->ShowError(error, false);
+    ShowArcSupportHostError(error, false);
     return;
   }
 
@@ -330,8 +329,7 @@ void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
 
   // We'll delay shutting down the ARC instance in this case to allow people
   // to send feedback.
-  if (support_host_)
-    support_host_->ShowError(error, true /* = show send feedback button */);
+  ShowArcSupportHostError(error, true /* = show send feedback button */);
 }
 
 void ArcSessionManager::SetState(State state) {
@@ -528,9 +526,9 @@ void ArcSessionManager::RequestEnableImpl() {
     // Check Android management in parallel.
     // Note: StartBackgroundAndroidManagementCheck() may call
     // OnBackgroundAndroidManagementChecked() synchronously (or
-    // asynchornously). In the callback, Google Play Store enabled preference
+    // asynchronously). In the callback, Google Play Store enabled preference
     // can be set to false if managed, and it triggers RequestDisable() via
-    // ArcPlayStoreEnabledPreferenceHandler.
+    // ArcPlayStoreEnabledPreferenceHandler
     // Thus, StartArc() should be called so that disabling should work even
     // if synchronous call case.
     StartBackgroundAndroidManagementCheck();
@@ -560,7 +558,7 @@ void ArcSessionManager::RequestDisable() {
 void ArcSessionManager::RequestArcDataRemoval() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
-  // TODO(hidehiko): DCHECK the previous state. This is called for three cases;
+  // TODO(hidehiko): DCHECK the previous state. This is called for four cases;

[hidehiko, 2017/03/13 05:10:46]

Good catch :-)

   // 1) Supporting managed user initial disabled case (Please see also
   //    ArcPlayStoreEnabledPreferenceHandler::Start() for details).
   // 2) Supporting enterprise triggered data removal.
@@ -599,10 +597,8 @@ void ArcSessionManager::MaybeStartTermsOfServiceNegotiation() {
     // If the user attempts to re-enable ARC while the ARC instance is still
     // running the user should not be able to continue until the ARC instance
     // has stopped.
-    if (support_host_) {
-      support_host_->ShowError(
-          ArcSupportHost::Error::SIGN_IN_SERVICE_UNAVAILABLE_ERROR, false);
-    }
+    ShowArcSupportHostError(
+        ArcSupportHost::Error::SIGN_IN_SERVICE_UNAVAILABLE_ERROR, false);
     return;
   }
 
@@ -699,6 +695,9 @@ void ArcSessionManager::StartAndroidManagementCheck() {
     support_host_->ShowArcLoading();
   }
 
+  for (auto& observer : observer_list_)
+    observer.OnArcStartOptInAndroidManagementCheck();
+
   android_management_checker_ = base::MakeUnique<ArcAndroidManagementChecker>(
       profile_, context_->token_service(), context_->account_id(),
       false /* retry_on_error */);
@@ -725,17 +724,13 @@ void ArcSessionManager::OnAndroidManagementChecked(
       StartArc();
       break;
     case policy::AndroidManagementClient::Result::MANAGED:
-      if (support_host_) {
-        support_host_->ShowError(
-            ArcSupportHost::Error::ANDROID_MANAGEMENT_REQUIRED_ERROR, false);
-      }
+      ShowArcSupportHostError(
+          ArcSupportHost::Error::ANDROID_MANAGEMENT_REQUIRED_ERROR, false);
       UpdateOptInCancelUMA(OptInCancelReason::ANDROID_MANAGEMENT_REQUIRED);
       break;
     case policy::AndroidManagementClient::Result::ERROR:
-      if (support_host_) {
-        support_host_->ShowError(
-            ArcSupportHost::Error::SERVER_COMMUNICATION_ERROR, false);
-      }
+      ShowArcSupportHostError(ArcSupportHost::Error::SERVER_COMMUNICATION_ERROR,
+                              false);
       UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
       break;
   }
@@ -956,6 +951,15 @@ void ArcSessionManager::SetAttemptUserExitCallbackForTesting(
   attempt_user_exit_callback_ = callback;
 }
 
+void ArcSessionManager::ShowArcSupportHostError(
+    ArcSupportHost::Error error,
+    bool should_show_send_feedback) {
+  if (support_host_)
+    support_host_->ShowError(error, should_show_send_feedback);
+  for (auto& observer : observer_list_)
+    observer.OnArcSupportHostErrorShown(error);

[hidehiko, 2017/03/13 05:10:46]

Notifying when |support_host_| is null looks weird?
Maybe either;
- Notify only |support_host_|->ShowError() is called, or
- Change the observer name and its concept to indicate that better.

[emaxx, 2017/03/16 18:32:12]

Done.
Renamed to OnArcShowingErrorRequested, do you think this name is clear enough?

+}
+
 std::ostream& operator<<(std::ostream& os,
                          const ArcSessionManager::State& state) {
 #define MAP_STATE(name)                \