Split IsArcPlayStoreEnabled() and ArcSessionManager's enabled concepts.

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 7c97cdc1aaabc2c93fa74e8e5700f3ff54e016fe..fd5142313899e243e9798a2458658a701d8f265e 100644
--- a/chrome/browser/chromeos/arc/arc_session_manager.h
+++ b/chrome/browser/chromeos/arc/arc_session_manager.h
@@ -170,6 +170,19 @@ class ArcSessionManager : public ArcSessionObserver,
   // Google Play Store, then ARC can run without opt-in.
   void SetArcPlayStoreEnabled(bool enable);
 
+  // Enables ARC session. This starts ARC instance, or maybe starts Terms Of
+  // Service negotiation if they haven't been accepted yet.
+  // If it is already enabled, no-op.
+  // Currently, enabled/disabled is tied to whether Google Play Store is

[Luis Héctor Chávez, 2017/02/15 19:18:53]

Maybe add a TODO?

[hidehiko, 2017/02/15 19:45:12]

Instead, referred TODO at SetArcPlayStoreEnabled().

+  // enabled or disabled.
+  void Enable();

[Luis Héctor Chávez, 2017/02/15 19:18:53]

I find it a bit inconsistent that we have SetArcPlayStoreEnabled(bool) and
Enable()/Disable(). Is there any compelling reason why it's not
SetArcEnabled(bool)?

[hidehiko, 2017/02/15 19:45:12]

No big reason, but I just followed RequestStart/Stop style used in
ArcSessionRunner, because we will have similar structure.

[Luis Héctor Chávez, 2017/02/15 21:54:43]

Given that we want to mirror that, should this be RequestEnable()? That way it
also reflects the nature that it does not necessarily enable it (because of the
ToS negotiation).

[hidehiko, 2017/02/16 02:44:28]

Done.

+
+  // Disables ARC session. This stops ARC instance, or quits Terms Of Service
+  // negotiation if it is the middle of the process (e.g. closing UI for
+  // manual negotiation if it is shown).
+  // If it is already disabled, no-op.
+  void Disable();
+
   // Called from the Chrome OS metrics provider to record Arc.State
   // periodically.
   void RecordArcState();
@@ -222,6 +235,10 @@ class ArcSessionManager : public ArcSessionObserver,
   void SetAttemptUserExitCallbackForTesting(const base::Closure& callback);
 
  private:
+  // Enable() has a check in order not to trigger starting procedure twice.
+  // This method can be called to bypass that check when restarting.
+  void EnableImpl();
+
   // Negotiates the terms of service to user.
   void StartTermsOfServiceNegotiation();
   void OnTermsOfServiceNegotiated(bool accepted);
@@ -260,6 +277,7 @@ class ArcSessionManager : public ArcSessionObserver,
   // Registrar used to monitor ARC enabled state.
   PrefChangeRegistrar pref_change_registrar_;
 
+  bool enabled_ = false;
   State state_ = State::NOT_INITIALIZED;
   base::ObserverList<Observer> observer_list_;
   base::ObserverList<ArcSessionObserver> arc_session_observer_list_;