Do nothing on OnSessionStopped if ARC is being restarted.

chrome/browser/chromeos/arc/arc_session_manager.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_ARC_SESSION_MANAGER_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_ARC_SESSION_MANAGER_H_
 
 #include <memory>
 #include <ostream>
 #include <string>
@@ skipping 67 matching lines @@
   // relationship with |enable_requested_|.
   enum class State {
     NOT_INITIALIZED,
     STOPPED,
     SHOWING_TERMS_OF_SERVICE,
     CHECKING_ANDROID_MANAGEMENT,
     REMOVING_DATA_DIR,
     ACTIVE,
   };
 
   class Observer {

[Yusuke Sato, 2017/02/28 21:07:17]

nit: Probably it's a good idea to have a short comment for the class now. Who
should use the observer and when?

As you know, similar events are relayed from ArcSession to ArcSessionRunner to
ArcSessionManager to its observers (with slightly different semantics, and now
with slightly different observer interfaces too), which is kind of confusing
without comments.

[hidehiko, 2017/03/01 09:10:04]

Done.

    public:
+    using StopReason = ArcSessionObserver::StopReason;
+
     virtual ~Observer() = default;

[Yusuke Sato, 2017/02/28 21:07:16]

nit: (while you're at it) isn't it better to make this pure virtual to prevent
Observer from being instantiated?

[hidehiko, 2017/03/01 09:10:04]

For that purpose, protected ctor looks more straightforward.
Also, I moved dtor to protected (with removing virtual) because we do not expect
the class is removed via Observer interface.
cf) http://www.gotw.ca/publications/mill18.htm

WDYT?

[hidehiko, 2017/03/01 11:19:03]

FYI: Found a recent discussion.
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/5GIek4TYiAw

 
     // Called to notify that whether Google Play Store is enabled or not, which
     // is represented by "arc.enabled" preference, is updated.
     virtual void OnArcPlayStoreEnabledChanged(bool enabled) {}
 
     // Called to notify that ARC has been initialized successfully.
     virtual void OnArcInitialStart() {}
 
+    // Called when ARC session is stopped, and is not being restarted
+    // automatically.
+    virtual void OnArcSessionStopped(StopReason stop_reason) {}
+
     // Called to notify that Android data has been removed. Used in
     // browser_tests
     virtual void OnArcDataRemoved() {}
   };
 
   explicit ArcSessionManager(
       std::unique_ptr<ArcSessionRunner> arc_session_runner);
   ~ArcSessionManager() override;
 
   static ArcSessionManager* Get();
@@ skipping 17 matching lines @@
 
   Profile* profile() { return profile_; }
   const Profile* profile() const { return profile_; }
 
   State state() const { return state_; }
 
   // Adds or removes observers.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // Adds or removes ArcSessionObservers.

[Yusuke Sato, 2017/02/28 21:07:16]

nice.

[hidehiko, 2017/03/01 09:10:04]

:-)

-  // TODO(hidehiko): The observer should be migrated into
-  // ArcSessionManager::Observer.
-  void AddSessionObserver(ArcSessionObserver* observer);
-  void RemoveSessionObserver(ArcSessionObserver* observer);
-
   // Returns true if ARC instance is running/stopped, respectively.
   // See ArcSessionRunner::IsRunning()/IsStopped() for details.
   bool IsSessionRunning() const;
   bool IsSessionStopped() const;
 
   // Called from ARC support platform app when user cancels signing.
   void CancelAuthCode();
 
   // Requests to enable ARC session. This starts ARC instance, or maybe starts
   // Terms Of Service negotiation if they haven't been accepted yet.
@@ skipping 93 matching lines @@
   // re-auth flow.
   void OnAndroidManagementChecked(
       policy::AndroidManagementClient::Result result);
 
   // Called when the background Android management check is done. It is
   // triggered when the second or later ARC boot timing.
   void OnBackgroundAndroidManagementChecked(
       policy::AndroidManagementClient::Result result);
 
   // ArcSessionObserver:
-  void OnSessionReady() override;
-  void OnSessionStopped(StopReason reason) override;
+  void OnSessionStopped(StopReason reason, bool restarting) override;
 
   std::unique_ptr<ArcSessionRunner> arc_session_runner_;
 
   // Unowned pointer. Keeps current profile.
   Profile* profile_ = nullptr;
 
   // Registrar used to monitor ARC enabled state.
   PrefChangeRegistrar pref_change_registrar_;
 
   // Whether ArcSessionManager is requested to enable (starting to run ARC
   // instance) or not.
   bool enable_requested_ = false;
 
   // Internal state machine. See also State enum class.
   State state_ = State::NOT_INITIALIZED;
   base::ObserverList<Observer> observer_list_;
-  base::ObserverList<ArcSessionObserver> arc_session_observer_list_;
   std::unique_ptr<ArcAppLauncher> playstore_launcher_;
   bool reenable_arc_ = false;
   bool provisioning_reported_ = false;
   base::OneShotTimer arc_sign_in_timer_;
 
   std::unique_ptr<ArcSupportHost> support_host_;
 
   std::unique_ptr<ArcTermsOfServiceNegotiator> terms_of_service_negotiator_;
 
   std::unique_ptr<ArcAuthContext> context_;
@@ skipping 11 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ArcSessionManager);
 };
 
 // Outputs the stringified |state| to |os|. This is only for logging purposes.
 std::ostream& operator<<(std::ostream& os,
                          const ArcSessionManager::State& state);
 
 }  // namespace arc
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_ARC_SESSION_MANAGER_H_