Extract kArcEnabled preference from ArcSessionManager part 2.

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>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
 #include "chrome/browser/chromeos/policy/android_management_client.h"
 #include "components/arc/arc_session_observer.h"
-#include "components/prefs/pref_change_registrar.h"
-#include "components/sync_preferences/pref_service_syncable_observer.h"
 
 class ArcAppLauncher;
 class Profile;
 
 namespace user_prefs {
 class PrefRegistrySyncable;
 }
 
 namespace arc {
 
 class ArcAndroidManagementChecker;
-class ArcAuthInfoFetcher;
 class ArcAuthContext;
 class ArcSessionRunner;
 class ArcTermsOfServiceNegotiator;
 enum class ProvisioningResult : int;
 
 // This class proxies the request from the client to fetch an auth code from
 // LSO. It lives on the UI thread.
 class ArcSessionManager : public ArcSessionObserver,
-                          public ArcSupportHost::Observer,
-                          public sync_preferences::PrefServiceSyncableObserver {
+                          public ArcSupportHost::Observer {
  public:
   // Represents each State of ARC session.
   // NOT_INITIALIZED: represents the state that the Profile is not yet ready
   //   so that this service is not yet initialized, or Chrome is being shut
   //   down so that this is destroyed.
   // STOPPED: ARC session is not running, or being terminated.
   // SHOWING_TERMS_OF_SERVICE: "Terms Of Service" page is shown on ARC support
   //   Chrome app.
   // CHECKING_ANDROID_MANAGEMENT: Checking Android management status. Note that
   //   the status is checked for each ARC session starting, but this is the
@@ skipping 72 matching lines @@
 
   // Sets the |profile|, and sets up Profile related fields in this instance.
   // IsArcAllowedForProfile() must return true for the given |profile|.
   void SetProfile(Profile* profile);
 
   Profile* profile() { return profile_; }
   const Profile* profile() const { return profile_; }
 
   State state() const { return state_; }
 
-  // Starts observing Google Play Store enabled preference change.
-  // Also, based on its initial value, this may start ArcSession, or may start
-  // removing the data, as initial state.
-  // In addition, this triggers to show ArcAuthNotification, if necessary.
-  // Note that this must be called after SetProfile().
-  // TODO(hidehiko): Extract preference related code into a class to split the
-  // dependencty.
-  void StartPreferenceHandler();
-
   // Adds or removes observers.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
+  // Notifies observers that Google Play Store enabled preference is changed.
+  // Note: ArcPlayStoreEnabledPreferenceHandler has the main responsibility to
+  // notify the event. However, due to life time, it is difficult for non-ARC
+  // services to subscribe the handler instance directly. Instead, they can to

[Luis Héctor Chávez, 2017/03/02 00:09:17]

nit: Instead, they can subscribe to

[hidehiko, 2017/03/02 02:21:57]

Done.

+  // ArcSessionManager, and ArcSessionManager proxies the event.
+  void NotifyArcPlayStoreEnabledChanged(bool enabled);
+
   // Adds or removes ArcSessionObservers.
   // 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;
@@ skipping 12 matching lines @@
   // Requests to disable 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 requested to disable, no-op.
   void RequestDisable();
 
   // 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,
                      bool is_backup_and_restore_enabled,
                      bool is_location_service_enabled) override;
   void OnRetryClicked() override;
   void OnSendFeedbackClicked() override;
 
   // Stops ARC without changing ArcEnabled preference.
   void StopArc();
 
   // StopArc(), then restart. Between them data clear may happens.
   // This is a special method to support enterprise device lost case.
   // This can be called only when ARC is running.
   void StopAndEnableArc();
 
   // Removes the data if ARC is stopped. Otherwise, queue to remove the data
   // on ARC is stopped. A log statement with the removal reason must be added
   // prior to calling RemoveArcData().
   void RemoveArcData();
 
   ArcSupportHost* support_host() { return support_host_.get(); }
 
   // TODO(hidehiko): Get rid of the getter by migration between ArcAuthContext
   // and ArcAuthInfoFetcher.
   ArcAuthContext* auth_context() { return context_.get(); }
 
   void StartArc();

[Yusuke Sato, 2017/03/01 18:09:27]

While you're at it, can you give a function comment to this? We have both
RequestEnable/Disable() and Start/StopArc(), plus StopAndEnableArc() as a bonus.
I guess it's better to have a full documentation here.

[hidehiko, 2017/03/01 19:21:31]

Done in another CL.

 
   void OnProvisioningFinished(ProvisioningResult result);

[Yusuke Sato, 2017/03/01 18:09:27]

same

[hidehiko, 2017/03/01 19:21:31]

Ditto.

 
   // Returns the time when the sign in process started, or a null time if
   // signing in didn't happen during this session.
   base::Time sign_in_start_time() const { return sign_in_start_time_; }
 
   // 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.
@@ skipping 13 matching lines @@
   // Negotiates the terms of service to user.
   void StartTermsOfServiceNegotiation();
   void OnTermsOfServiceNegotiated(bool accepted);
 
   // Returns whether ARC is managed and all ARC related OptIn preferences are
   // managed too.
   bool AreArcAllOptInPreferencesManaged() const;
 
   void SetState(State state);
   void ShutdownSession();
-  void OnOptInPreferenceChanged();
   void OnAndroidManagementPassed();
   void OnArcDataRemoved(bool success);
   void OnArcSignInTimeout();
 
   void StartArcAndroidManagementCheck();
   void MaybeReenableArc();
 
   // Called when the Android management check is done in opt-in flow or
   // 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;
 
   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;
@@ skipping 19 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_