Turn ArcSupportHost from Observer model to Delegate

chrome/browser/chromeos/arc/arc_support_host.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_SUPPORT_HOST_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_ARC_SUPPORT_HOST_H_
 
 #include <memory>
 #include <string>
 
@@ skipping 26 matching lines @@
     NETWORK_UNAVAILABLE_ERROR,
     SERVER_COMMUNICATION_ERROR,
     SIGN_IN_BAD_AUTHENTICATION_ERROR,
     SIGN_IN_CLOUD_PROVISION_FLOW_FAIL_ERROR,
     SIGN_IN_GMS_NOT_AVAILABLE_ERROR,
     SIGN_IN_NETWORK_ERROR,
     SIGN_IN_SERVICE_UNAVAILABLE_ERROR,
     SIGN_IN_UNKNOWN_ERROR,
   };
 
-  // Observer to notify UI event.
-  class Observer {
+  // Delegate to handle manual authentication related events. The life cycle of

[Luis Héctor Chávez, 2017/05/12 15:41:30]

All the "The life cycle of the instance..." comments are maybe unnecessary,
since they are responsible for their own lifetime.

[victorhsieh0, 2017/05/12 18:31:55]

Removed, but add extra comment in the relevant code, which implicitly relies on
the pointer availability.

+  // the instance is as short as the manual authentication process.
+  class AuthDelegate {
    public:
-    virtual ~Observer() = default;
+    virtual ~AuthDelegate() = default;
 
-    // Called when the ARC support window is closed.
-    virtual void OnWindowClosed() {}
+    // Called when LSO auth token fetch is successfully completed.
+    virtual void OnAuthSucceeded(const std::string& auth_code) = 0;
 
-    // Called when the user press AGREE button on ToS page.
+    // Called when LSO auth token fetch has failed.
+    virtual void OnAuthFailed() = 0;
+
+    // Called when "RETRY" button on the error page is clicked during
+    // authentication.
+    virtual void OnAuthRetry() = 0;
+  };
+
+  // Delegate to handle manual authentication related events. The life cycle of
+  // the instance is as short as the terms of service negotiation.
+  class TermsOfServiceDelegate {
+   public:
+    virtual ~TermsOfServiceDelegate() = default;
+
+    // Called when the user press AGREE button on terms of service page.
     virtual void OnTermsAgreed(bool is_metrics_enabled,
                                bool is_backup_and_restore_enabled,
-                               bool is_location_service_enabled) {}
+                               bool is_location_service_enabled) = 0;
 
-    // Called when LSO auth token fetch is successfully completed.
-    virtual void OnAuthSucceeded(const std::string& auth_code) {}
+    // Called when the user rejects the terms of service or close the page.
+    virtual void OnTermsRejected() = 0;
 
-    // Called when LSO auth token fetch has failed.
-    virtual void OnAuthFailed() {}
+    // Called when "RETRY" button on the error page is clicked during terms of
+    // service negotiation.
+    virtual void OnTermsRetry() = 0;

[Luis Héctor Chávez, 2017/05/12 15:41:31]

OnTermsRetried? OnTermsRetryClicked? to be consistent with the OnObjectVerbed
pattern in the rest of the method names. Same in L61.

[victorhsieh0, 2017/05/12 18:31:55]

Done.

+  };
 
-    // Called when "RETRY" button on the error page is clicked.
+  class ErrorDelegate {

[Luis Héctor Chávez, 2017/05/12 15:41:31]

Comment? At least mention that this is the "fallback" delegate that will be
invoked only if there is no more specific delegate.

[victorhsieh0, 2017/05/12 18:31:55]

Done.

+   public:
+    virtual ~ErrorDelegate() = default;
+
+    // Called when the opt-in process is aborted. This currently happens when
+    // the ARC support window is closed (except when when the terms of service
+    // negotiation is ongoing, the most specific function in
+    // TermsOfServiceDelegate is called).
+    virtual void OnOptInAborted() {}

[Luis Héctor Chávez, 2017/05/12 15:41:31]

Any reason why this Delegate has implementations, while the other two only have
pure virtual methods?

[victorhsieh0, 2017/05/12 18:31:55]

Done.

+
+    // Called when "RETRY" button on the error page is clicked, except when
+    // terms of service negotiation or manual authentication is onging. In those
+    // cases, the more specific retry function in the other delegates is called.
     virtual void OnRetryClicked() {}
 
     // Called when send feedback button on error page is clicked.
     virtual void OnSendFeedbackClicked() {}
   };
 
   static const char kHostAppId[];
   static const char kStorageId[];
 
   using RequestOpenAppCallback = base::Callback<void(Profile* profile)>;
 
   explicit ArcSupportHost(Profile* profile);
   ~ArcSupportHost() override;
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
   bool HasObserver(Observer* observer);
 
+  void SetAuthDelegate(AuthDelegate* delegate);
+  void SetTermsOfServiceDelegate(TermsOfServiceDelegate* delegate);
+  void SetErrorDelegate(ErrorDelegate* delegate);
+
   // Called when the communication to arc_support Chrome App is ready.
   void SetMessageHost(arc::ArcSupportMessageHost* message_host);
 
   // Called when the communication to arc_support Chrome App is closed.
   // The argument message_host is used to check if the given |message_host|
   // is what this instance uses know, to avoid racy case.
   // If |message_host| is different from the one this instance knows,
   // this is no op.
   void UnsetMessageHost(arc::ArcSupportMessageHost* message_host);
 
@@ skipping 64 matching lines @@
   // }
   void SendPreferenceCheckboxUpdate(const std::string& action_name,
                                     const PreferenceCheckboxData& data);
 
   void DisconnectMessageHost();
 
   Profile* const profile_;
   RequestOpenAppCallback request_open_app_callback_;
 
   base::ObserverList<Observer> observer_list_;
+  AuthDelegate* auth_delegate_ = nullptr;           // not owned
+  TermsOfServiceDelegate* tos_delegate_ = nullptr;  // not owned
+  ErrorDelegate* error_delegate_ = nullptr;         // not owned
 
   // True, if ARC support app is requested to start, but the connection is not
   // yet established. Reset to false, when the app is started and the
   // connection to the app is established.
   bool app_start_pending_ = false;
 
   // 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 have valid values iff ui_page_ == ERROR.
   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);
 };
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_ARC_SUPPORT_HOST_H_