Cleanups in remoting::HostStarter.

several minor cleanups in the HostStarter class:
 - Removed START_IN_PROGRESS result code and replaced it with DCHECK.
 - Removed in_progress_ flag.
 - Cleaned up how on_done_ callback is called - class members must not
   be used after the callback is called.
 - Added TODO for more cleanups.


Review URL: https://chromiumcodereview.appspot.com/11227062

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163893 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 31 insertions, 41 deletions

diff --git a/remoting/host/setup/host_starter.cc b/remoting/host/setup/host_starter.cc
index 063ce83..5904b63 100644
--- a/remoting/host/setup/host_starter.cc
+++ b/remoting/host/setup/host_starter.cc
@@ -26,7 +26,6 @@ HostStarter::HostStarter(
       user_email_fetcher_(user_email_fetcher.Pass()),
       service_client_(service_client.Pass()),
       daemon_controller_(daemon_controller.Pass()),
-      in_progress_(false),
       consent_to_data_collection_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
       weak_ptr_(weak_ptr_factory_.GetWeakPtr()) {
@@ -60,11 +59,8 @@ void HostStarter::StartHost(
     const std::string& redirect_url,
     CompletionCallback on_done) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
-  if (in_progress_) {
-    on_done.Run(START_IN_PROGRESS);
-    return;
-  }
-  in_progress_ = true;
+  DCHECK(on_done_.is_null());
+
   host_name_ = host_name;
   host_pin_ = host_pin;
   consent_to_data_collection_ = consent_to_data_collection;
@@ -95,6 +91,13 @@ void HostStarter::OnGetTokensResponse(
   user_email_fetcher_->GetUserEmail(access_token_, this);
 }
 
+void HostStarter::OnRefreshTokenResponse(
+    const std::string& access_token,
+    int expires_in_seconds) {
+  // We never request a refresh token, so this call is not expected.
+  NOTREACHED();
+}
+
 void HostStarter::OnGetUserEmailResponse(const std::string& user_email) {
   if (!main_task_runner_->BelongsToCurrentThread()) {
     main_task_runner_->PostTask(FROM_HERE, base::Bind(
@@ -135,12 +138,12 @@ void HostStarter::OnHostStarted(DaemonController::AsyncResult result) {
         &HostStarter::OnHostStarted, weak_ptr_, result));
     return;
   }
-  Result done_result = START_ERROR;
-  if (result == DaemonController::RESULT_OK)
-    done_result = START_COMPLETE;
-  on_done_.Run(done_result);
   // TODO(simonmorris): Unregister the host if we didn't start it.
-  in_progress_ = false;
+  Result done_result = (result == DaemonController::RESULT_OK) ?
+      START_COMPLETE : START_ERROR;
+  CompletionCallback cb = on_done_;
+  on_done_.Reset();
+  cb.Run(done_result);
 }
 
 void HostStarter::OnOAuthError() {
@@ -149,8 +152,9 @@ void HostStarter::OnOAuthError() {
         &HostStarter::OnOAuthError, weak_ptr_));
     return;
   }
-  on_done_.Run(OAUTH_ERROR);
-  in_progress_ = false;
+  CompletionCallback cb = on_done_;
+  on_done_.Reset();
+  cb.Run(OAUTH_ERROR);
 }
 
 void HostStarter::OnNetworkError(int response_code) {
@@ -159,22 +163,9 @@ void HostStarter::OnNetworkError(int response_code) {
         &HostStarter::OnNetworkError, weak_ptr_, response_code));
     return;
   }
-  on_done_.Run(NETWORK_ERROR);
-  in_progress_ = false;
-}
-
-void HostStarter::OnRefreshTokenResponse(
-    const std::string& access_token,
-    int expires_in_seconds) {
-  NOTREACHED();
-  if (!main_task_runner_->BelongsToCurrentThread()) {
-    main_task_runner_->PostTask(FROM_HERE, base::Bind(
-        &HostStarter::OnRefreshTokenResponse, weak_ptr_,
-        access_token, expires_in_seconds));
-    return;
-  }
-  on_done_.Run(OAUTH_ERROR);
-  in_progress_ = false;
+  CompletionCallback cb = on_done_;
+  on_done_.Reset();
+  cb.Run(NETWORK_ERROR);
 }
 
 }  // namespace remoting
diff --git a/remoting/host/setup/host_starter.h b/remoting/host/setup/host_starter.h
index 11b2df1..fa615c4 100644
--- a/remoting/host/setup/host_starter.h
+++ b/remoting/host/setup/host_starter.h
@@ -18,13 +18,11 @@
 namespace remoting {
 
 // A helper class that registers and starts a host.
-class HostStarter :
-    public gaia::GaiaOAuthClient::Delegate,
-    public remoting::GaiaUserEmailFetcher::Delegate,
-    public remoting::ServiceClient::Delegate {
+class HostStarter : public gaia::GaiaOAuthClient::Delegate,
+                    public remoting::GaiaUserEmailFetcher::Delegate,
+                    public remoting::ServiceClient::Delegate {
  public:
-  enum Result{
-    START_IN_PROGRESS,
+  enum Result {
     START_COMPLETE,
     NETWORK_ERROR,
     OAUTH_ERROR,
@@ -55,8 +53,6 @@ class HostStarter :
                                    int expires_in_seconds) OVERRIDE;
   virtual void OnRefreshTokenResponse(const std::string& access_token,
                                       int expires_in_seconds) OVERRIDE;
-  virtual void OnOAuthError() OVERRIDE;
-  virtual void OnNetworkError(int response_code) OVERRIDE;
 
   // remoting::GaiaUserEmailFetcher::Delegate
   virtual void OnGetUserEmailResponse(const std::string& user_email) OVERRIDE;
@@ -64,6 +60,13 @@ class HostStarter :
   // remoting::ServiceClient::Delegate
   virtual void OnHostRegistered() OVERRIDE;
 
+  // TODO(sergeyu): Following methods are members of all three delegate
+  // interfaces implemented in this class. Fix ServiceClient and
+  // GaiaUserEmailFetcher so that Delegate interfaces do not overlap (ideally
+  // they should be changed to use Callback<>).
+  virtual void OnOAuthError() OVERRIDE;
+  virtual void OnNetworkError(int response_code) OVERRIDE;
+
  private:
   HostStarter(scoped_ptr<gaia::GaiaOAuthClient> oauth_client,
               scoped_ptr<remoting::GaiaUserEmailFetcher> user_email_fetcher,
@@ -76,7 +79,6 @@ class HostStarter :
   scoped_ptr<remoting::GaiaUserEmailFetcher> user_email_fetcher_;
   scoped_ptr<remoting::ServiceClient> service_client_;
   scoped_ptr<remoting::DaemonController> daemon_controller_;
-  bool in_progress_;
   gaia::OAuthClientInfo oauth_client_info_;
   std::string host_name_;
   std::string host_pin_;
diff --git a/remoting/host/setup/start_host.cc b/remoting/host/setup/start_host.cc
index 07db748..a089942 100644
--- a/remoting/host/setup/start_host.cc
+++ b/remoting/host/setup/start_host.cc
@@ -65,9 +65,6 @@ void OnDone(HostStarter::Result result) {
     return;
   }
   switch (result) {
-    case HostStarter::START_IN_PROGRESS:
-      fprintf(stderr, "Internal error: START_IN_PROGRESS.\n");
-      break;
     case HostStarter::START_COMPLETE:
       g_started = true;
       break;