Split IsArcPlayStoreEnabled() and ArcSessionManager's enabled concepts.

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 152 matching lines @@
   bool IsArcPlayStoreEnabled() const;
 
   // Enables/disables Google Play Store on ARC. Currently, it is tied to
   // ARC enabled state, too, so this also should trigger to enable/disable
   // whole ARC system.
   // TODO(hidehiko): De-couple the concept to enable ARC system and opt-in
   // to use Google Play Store. Note that there is a plan to use ARC without
   // 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();
 
   // sync_preferences::PrefServiceSyncableObserver
   void OnIsSyncingChanged() override;
 
   // ArcSupportHost::Observer:
   void OnWindowClosed() override;
   void OnTermsAgreed(bool is_metrics_enabled,
@@ skipping 32 matching lines @@
   // Returns the time when ARC was about to start, or a null time if ARC has not
   // been started yet.
   base::Time arc_start_time() const { return arc_start_time_; }
 
   // Injectors for testing.
   void SetArcSessionRunnerForTesting(
       std::unique_ptr<ArcSessionRunner> arc_session_runner);
   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);
 
   void SetState(State state);
   void ShutdownSession();
   void OnOptInPreferenceChanged();
   void OnAndroidManagementPassed();
   void OnArcDataRemoved(bool success);
   void OnArcSignInTimeout();
@@ skipping 18 matching lines @@
   void OnSessionStopped(StopReason reason) 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_;
 
+  bool enabled_ = false;
   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_;
 
@@ skipping 14 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_