Do nothing on OnSessionStopped if ARC is being restarted.

chrome/browser/chromeos/arc/arc_session_manager.h

Index: chrome/browser/chromeos/arc/arc_session_manager.h
diff --git a/chrome/browser/chromeos/arc/arc_session_manager.h b/chrome/browser/chromeos/arc/arc_session_manager.h
index d5192354ba0826e00b869fefa7bd31eb77a04cc7..cc9fddfd50baa3e54c6845bafff00ec489207cbc 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.h
+++ b/chrome/browser/chromeos/arc/arc_session_manager.h
@@ -87,6 +87,8 @@ class ArcSessionManager : public ArcSessionObserver,
 
   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
@@ -96,6 +98,10 @@ class ArcSessionManager : public ArcSessionObserver,
     // 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() {}
@@ -133,12 +139,6 @@ class ArcSessionManager : public ArcSessionObserver,
   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;
@@ -252,8 +252,7 @@ class ArcSessionManager : public ArcSessionObserver,
       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_;
 
@@ -270,7 +269,6 @@ class ArcSessionManager : public ArcSessionObserver,
   // 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;