Do nothing on OnSessionStopped if ARC is being restarted.

chrome/browser/chromeos/arc/arc_session_manager.cc

 // Copyright 2016 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_session_manager.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 124 matching lines @@
   // TODO(hidehiko): When the dependency to ArcAuthNotification from this
   // class is removed, we should remove this as well.
   ArcAuthNotification::DisableForTesting();
 }
 
 // static
 void ArcSessionManager::EnableCheckAndroidManagementForTesting() {
   g_enable_check_android_management_for_testing = true;
 }
 
-void ArcSessionManager::OnSessionReady() {
-  for (auto& observer : arc_session_observer_list_)
-    observer.OnSessionReady();
-}
+void ArcSessionManager::OnSessionStopped(StopReason reason, bool restarting) {
+  if (restarting) {
+    // If ARC is being restarted, here do nothing, and just wait for its
+    // next run.
+    VLOG(1) << "ARC session is stopped, but being restarted: " << reason;
+    return;
+  }
 
-void ArcSessionManager::OnSessionStopped(StopReason reason) {
   // TODO(crbug.com/625923): Use |reason| to report more detailed errors.
   if (arc_sign_in_timer_.IsRunning())

[hidehiko, 2017/02/28 16:33:48]

Note: this case is the biggest change in this CL.
If ARC is somehow crashed and then being restarted in this case, restarting will
get priority with this change.
Even in the crash loop case, as a last resort, timeout should be fired.

Another approach would be, if the timer is running,
stop the ArcSessionRunner, even in restarting case.

Luis, WDYT?

     OnProvisioningFinished(ProvisioningResult::ARC_STOPPED);
 
   if (profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested)) {

[hidehiko, 2017/02/28 16:33:48]

Note: in case of restarting, we do not want to enter data removal state.

     // This should be always true, but just in case as this is looked at
     // inside RemoveArcData() at first.
     VLOG(1) << "ARC had previously requested to remove user data.";
     DCHECK(arc_session_runner_->IsStopped());
     RemoveArcData();
   } else {
     // To support special "Stop and enable ARC" procedure for enterprise,
     // here call MaybeReenableArc() asyncronously.
     // TODO(hidehiko): Restructure the code. crbug.com/665316
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&ArcSessionManager::MaybeReenableArc,
                               weak_ptr_factory_.GetWeakPtr()));
   }
 
-  for (auto& observer : arc_session_observer_list_)
-    observer.OnSessionStopped(reason);
+  for (auto& observer : observer_list_)
+    observer.OnArcSessionStopped(reason);

[hidehiko, 2017/02/28 16:33:48]

Note: The only use case of this is ArcBootNotification, which sees LOW_DISK
error only.
On LOW_DISK error, restarting should always false (because it is a part of
container boot sequence), so this should be practically no change.

 }
 
 void ArcSessionManager::RemoveArcData() {
   // Ignore redundant data removal request.
   if (state() == State::REMOVING_DATA_DIR)
     return;
 
   // OnArcDataRemoved resets this flag.
   profile_->GetPrefs()->SetBoolean(prefs::kArcDataRemoveRequested, true);
 
@@ skipping 388 matching lines @@
 void ArcSessionManager::AddObserver(Observer* observer) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   observer_list_.AddObserver(observer);
 }
 
 void ArcSessionManager::RemoveObserver(Observer* observer) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   observer_list_.RemoveObserver(observer);
 }
 
-void ArcSessionManager::AddSessionObserver(ArcSessionObserver* observer) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  arc_session_observer_list_.AddObserver(observer);
-}
-
-void ArcSessionManager::RemoveSessionObserver(ArcSessionObserver* observer) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  arc_session_observer_list_.RemoveObserver(observer);
-}
-
 bool ArcSessionManager::IsSessionRunning() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return arc_session_runner_->IsRunning();
 }
 
 bool ArcSessionManager::IsSessionStopped() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return arc_session_runner_->IsStopped();
 }
 
@@ skipping 395 matching lines @@
 
 #undef MAP_STATE
 
   // Some compilers report an error even if all values of an enum-class are
   // covered exhaustively in a switch statement.
   NOTREACHED() << "Invalid value " << static_cast<int>(state);
   return os;
 }
 
 }  // namespace arc