arc: Fix double provisioning failure reporting.

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..bcaa3cb58376a59da3c07a26c80b33ba15f4ccae 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.cc
+++ b/chrome/browser/chromeos/arc/arc_session_manager.cc
@@ -194,6 +194,7 @@ void ArcSessionManager::OnBridgeStopped(ArcBridgeService::StopReason reason) {
   if (arc_sign_in_timer_.IsRunning()) {
     OnProvisioningFinished(ProvisioningResult::ARC_STOPPED);
   }
+  provisioning_error_reported_ = false;

[Luis Héctor Chávez, 2016/12/06 17:35:20]

Is this one needed? Shouldn't we just use the one in StartArc()?

[khmel, 2016/12/06 17:46:55]

Actually not, more paranoiac...

 
   if (clear_required_) {
     // This should be always true, but just in case as this is looked at
@@ -248,6 +249,15 @@ void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::ACTIVE);
 
+  if (result != ProvisioningResult::SUCCESS) {

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

Per offline discussion, we want to guarantee OnProvisioningFinished() exactly
once per ArcSession run.
So, could you make it so?
- Removing this if condition.
- s/provisioning_error_reported_/provisioining_finished_called_/ or something?
- at L254: I think as this is unexpected behavior, NOTREACHED() should be
better.

[khmel, 2016/12/06 18:13:35]

Per our discussion I thought that we want to have something like
ExactlyOneCallback. So check is needed here.

[hidehiko, 2016/12/06 18:15:54]

I meant, it is not needed to check if result is SUCCESS or not.
If this is called or not is still needed, as you said.

[khmel, 2016/12/06 18:41:13]

Got you! However LOG(WARNING) sounds better because this is actual case and we
really handle it. Also added check for SUCCESS

+    if (provisioning_error_reported_) {
+      LOG(WARNING) << " Provisioning error was already reported. Ignoring "
+                   << " additional error " << static_cast<int>(result) << ".";
+      return;
+    }
+    provisioning_error_reported_ = true;
+  }
+
   if (result == ProvisioningResult::CHROME_SERVER_COMMUNICATION_ERROR) {
     if (IsArcKioskMode()) {
       VLOG(1) << "Robot account auth code fetching error";
@@ -591,6 +601,7 @@ void ArcSessionManager::StopAndEnableArc() {
 
 void ArcSessionManager::StartArc() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  provisioning_error_reported_ = false;
   arc_bridge_service()->RequestStart();
   SetState(State::ACTIVE);
 }