Refactor ArcSupportHost and ArcAuthService part2.

chrome/browser/chromeos/arc/arc_support_host.h

Index: chrome/browser/chromeos/arc/arc_support_host.h
diff --git a/chrome/browser/chromeos/arc/arc_support_host.h b/chrome/browser/chromeos/arc/arc_support_host.h
index c7dd72a08b8c533e2549a51f0a9130b8d134b4ea..3add540433360e6a7810a489244f4a96df518212 100644
--- a/chrome/browser/chromeos/arc/arc_support_host.h
+++ b/chrome/browser/chromeos/arc/arc_support_host.h
@@ -12,22 +12,32 @@
 #include "extensions/browser/api/messaging/native_message_host.h"
 #include "ui/display/display_observer.h"
 
+class Profile;
+
 // Native interface to control ARC support chrome App.
-// TODO(hidehiko): Move more implementation for the UI control from
-// ArcAuthService to this class.
 // TODO(hidehiko,lhchavez): Move this into extensions/ directory, and put it
 // into "arc" namespace. Add unittests at the time.
 class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
                        public display::DisplayObserver {
  public:
   enum class UIPage {
-    NO_PAGE,              // Hide everything.
-    TERMS,                // Terms content page.
-    LSO_PROGRESS,         // LSO loading progress page.
-    LSO,                  // LSO page to enter user's credentials.
-    START_PROGRESS,       // Arc starting progress page.
-    ERROR,                // Arc start error page.
-    ERROR_WITH_FEEDBACK,  // Arc start error page, plus feedback button.
+    NO_PAGE,      // Hide everything.
+    TERMS,        // Terms content page.
+    LSO,          // LSO page to enter user's credentials.
+    ARC_LOADING,  // ARC loading progress page.
+    ERROR,        // ARC start error page.
+  };
+
+  // Error types whose corresponding message ARC support has.
+  enum class Error {
+    SIGN_IN_NETWORK_ERROR,
+    SIGN_IN_SERVICE_UNAVAILABLE_ERROR,
+    SIGN_IN_BAD_AUTHENTICATION_ERROR,
+    SIGN_IN_GMS_NOT_AVAILABLE_ERROR,
+    SIGN_IN_CLOUD_PROVISION_FLOW_FAIL_ERROR,
+    SIGN_IN_UNKNOWN_ERROR,
+    SERVER_COMMUNICATION_ERROR,
+    ANDROID_MANAGEMENT_REQUIRED_ERROR,
   };
 
   // Observer to notify UI event.
@@ -58,7 +68,7 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
   static const char kHostAppId[];
   static const char kStorageId[];
 
-  ArcSupportHost();
+  explicit ArcSupportHost(Profile* profile);
   ~ArcSupportHost() override;
 
   // Currently only one observer can be added.
@@ -75,9 +85,24 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
   // this is no op.
   void UnsetMessageHost(arc::ArcSupportMessageHost* message_host);
 
+  // Sets the ARC managed state. This must be called before ARC support app
+  // is started.
+  void SetArcManaged(bool is_arc_managed);

[Luis Héctor Chávez, 2016/11/16 02:58:53]

It seems like this can be set in the constructor. That should reduce the number
of moving parts.

[hidehiko, 2016/11/16 17:52:21]

ARC managed status can be changed, and OnOptInPreferenceChanged() should be
called in that timing, so ctor does not work, IIUC? That's why this setter is
introduced.

[Luis Héctor Chávez, 2016/11/16 23:20:30]

argh, sorry, forgot that was possible.

+
   // Requests to close the extension window.
   void Close();
-  void ShowPage(UIPage page, const base::string16& status);
+
+  // Requests to show the "Terms Of Service" page.
+  void ShowTermsOfService();
+
+  // Requests to show the LSO page.
+  void ShowLso();
+
+  // Requests to show the "ARC is loadign" page.

[Luis Héctor Chávez, 2016/11/16 02:58:53]

nit: s/loadign/loading/

[hidehiko, 2016/11/16 17:52:21]

Done.

+  void ShowArcLoading();
+
+  // Requests to show the error page
+  void ShowError(Error error, bool should_show_send_feedback);
 
   void SetMetricsPreferenceCheckbox(bool is_enabled, bool is_managed);
   void SetBackupAndRestorePreferenceCheckbox(bool is_enabled, bool is_managed);
@@ -92,6 +117,11 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t changed_metrics) override;
 
+  // Returns current page that has to be shown in OptIn UI.
+  // Note that this can be inconsistent from the actually shown page.
+  // TODO(hidehiko): Remove this exposure.
+  UIPage ui_page() const { return ui_page_; }
+
  private:
   struct PreferenceCheckboxData {
     PreferenceCheckboxData() : PreferenceCheckboxData(false, false) {}
@@ -102,8 +132,14 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
     bool is_managed;
   };
 
+  // Requests to start the ARC support Chrome app.
+  void RequestAppStart();
+
   bool Initialize();
 
+  // Requests to ARC support Chrome app to show the specified page.
+  void ShowPage(UIPage ui_page);
+
   // Sends a preference update to the extension.
   // The message will be
   // {
@@ -116,17 +152,35 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
 
   void DisconnectMessageHost();
 
+  Profile* profile_;

[Luis Héctor Chávez, 2016/11/16 02:58:53]

nit: Profile* const profile_;

[hidehiko, 2016/11/16 17:52:21]

Done.

+
   // Currently Observer is only ArcAuthService, so it is unique.
   // Use ObserverList when more classes start to observe it.
   Observer* observer_ = nullptr;
 
-  PreferenceCheckboxData metrics_checkbox_;
-  PreferenceCheckboxData backup_and_restore_checkbox_;
-  PreferenceCheckboxData location_services_checkbox_;
+  // True, if ARC support app is requested to start. Reset to false,
+  // when the app is started and the connection to the app is
+  // established.
+  bool app_start_requested_ = false;

[Luis Héctor Chávez, 2016/11/16 02:58:53]

How about |app_start_pending_|? That better reflects the transient nature of
this resettable flag.

[hidehiko, 2016/11/16 17:52:21]

Done.

 
   // The instance is created and managed by Chrome.
   arc::ArcSupportMessageHost* message_host_ = nullptr;
 
+  // The lifetime of the message_host_ is out of control from ARC.
+  // Fields below are UI parameter cache in case the value is set before
+  // connection to the ARC support Chrome app is established.
+  UIPage ui_page_ = UIPage::NO_PAGE;
+
+  // These are valid value iff ui_page_ == ERROR.

[Luis Héctor Chávez, 2016/11/16 02:58:53]

nit: s/are valid values/have valid values/.

[hidehiko, 2016/11/16 17:52:21]

Done.

+  Error error_;
+  bool should_show_send_feedback_;
+
+  bool is_arc_managed_ = false;
+
+  PreferenceCheckboxData metrics_checkbox_;
+  PreferenceCheckboxData backup_and_restore_checkbox_;
+  PreferenceCheckboxData location_services_checkbox_;
+
   DISALLOW_COPY_AND_ASSIGN(ArcSupportHost);
 };