Turn ArcSupportHost from Observer model to Delegate

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 c645e59073561f2a658f440d7f8590944b432354..2834b2fecd99a452894ba6e1112a4daf50cffb86 100644
--- a/chrome/browser/chromeos/arc/arc_support_host.h
+++ b/chrome/browser/chromeos/arc/arc_support_host.h
@@ -44,27 +44,48 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
     SIGN_IN_UNKNOWN_ERROR,
   };
 
-  // Observer to notify UI event.
-  class Observer {
+  class AuthHandler {
    public:
-    virtual ~Observer() = default;
+    virtual ~AuthHandler() = 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 LSO auth token fetch has failed.
+    virtual void OnAuthFailed() = 0;
+
+    // Called when the user clicks retry.
+    virtual void OnAuthRetry() = 0;
+  };
+
+  class TosHandler {
+   public:
+    virtual ~TosHandler() = default;
 
     // Called when the user press AGREE button on ToS 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 ToS or close the page.
+    virtual void OnTermsRejected() = 0;
 
-    // Called when LSO auth token fetch has failed.
-    virtual void OnAuthFailed() {}
+    // Called when the user click "RETRY" button on ToS error.
+    virtual void OnTermsError() = 0;
+  };
+
+  class ErrorHandler {
+   public:
+    virtual ~ErrorHandler() = default;
 
-    // Called when "RETRY" button on the error page is clicked.
-    virtual void OnRetryClicked() {}
+    // Called when the ARC support window is closed, except when ToS negotiation
+    // is ongoing, TosHandler::OnTermsRejected() will be called.
+    virtual void OnWindowClosed() {}  // FIXME
+
+    // Called when "RETRY" button on the error page is clicked, except when ToS
+    // negotiation is ongoing.  In that case, TosHandler::OnTermsError() will be
+    // called.
+    virtual void OnRetryClicked() {}  // TODO rename to match OnTermsError?
 
     // Called when send feedback button on error page is clicked.
     virtual void OnSendFeedbackClicked() {}
@@ -82,6 +103,15 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
   void RemoveObserver(Observer* observer);
   bool HasObserver(Observer* observer);
 
+  void SetAuthHandler(AuthHandler* handler);

[Luis Héctor Chávez, 2017/05/01 16:45:56]

Probably a naming nit, but typically Handlers are chained:
https://en.wikipedia.org/wiki/Chain-of-responsibility_pattern ,
https://cs.chromium.org/chromium/src/chrome/service/service_ipc_server.h?l=66

These all look like Delegates to me.

[victorhsieh0, 2017/05/11 23:51:36]

Done.

+  void UnsetAuthHandler();

[Luis Héctor Chávez, 2017/05/01 16:45:56]

Maybe remove all the Unset*() and just call SetXxxYyy(nullptr)?

[victorhsieh0, 2017/05/11 23:51:36]

Done.

+
+  void SetTosHandler(TosHandler* handler);
+  void UnsetTosHandler();
+
+  void SetErrorHandler(ErrorHandler* handler);
+  void UnsetErrorHandler();
+
   // Called when the communication to arc_support Chrome App is ready.
   void SetMessageHost(arc::ArcSupportMessageHost* message_host);
 
@@ -166,6 +196,9 @@ class ArcSupportHost : public arc::ArcSupportMessageHost::Observer,
   RequestOpenAppCallback request_open_app_callback_;
 
   base::ObserverList<Observer> observer_list_;
+  AuthHandler* auth_handler_ = nullptr;  // not owned
+  TosHandler* tos_handler_ = nullptr;  // not owned
+  ErrorHandler* error_handler_ = nullptr;  // not owned

[Luis Héctor Chávez, 2017/05/01 16:45:56]

Should you DCHECK if any of these are non-nullptr on desruction? That'll signal
that there was a race / destruction order SNAFU.

[victorhsieh0, 2017/05/11 23:51:36]

Done.

 
   // 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