Move OAuth token refreshing to SignalingConnector.

BUG=118928


Review URL: http://codereview.chromium.org/9956161

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

7 files changed, 159 insertions, 216 deletions

diff --git a/remoting/host/oauth_client.cc b/remoting/host/oauth_client.cc
deleted file mode 100644
index 8156bfd..0000000
--- a/remoting/host/oauth_client.cc
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright (c) 2012 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.
-
-#include "remoting/host/oauth_client.h"
-
-#include "base/bind.h"
-#include "remoting/host/chromoting_host_context.h"
-#include "remoting/host/url_request_context.h"
-
-namespace remoting {
-
-OAuthClient::OAuthClient()
-    : delegate_(NULL) {
-}
-
-OAuthClient::~OAuthClient() {
-}
-
-void OAuthClient::Start(
-    const scoped_refptr<URLRequestContextGetter>& url_context_,
-    const std::string& refresh_token,
-    OAuthClient::Delegate* delegate,
-    base::MessageLoopProxy* message_loop) {
-  gaia_oauth_client_.reset(
-      new GaiaOAuthClient(kGaiaOAuth2Url, url_context_));
-  refresh_token_ = refresh_token;
-  delegate_ = delegate;
-  message_loop_ = message_loop;
-  RefreshToken();
-}
-
-void OAuthClient::OnGetTokensResponse(const std::string& refresh_token,
-                                      const std::string& access_token,
-                                      int expires_in_seconds) {
-  NOTREACHED();
-}
-
-void OAuthClient::OnRefreshTokenResponse(const std::string& access_token,
-                                         int expires_in_seconds) {
-  message_loop_->PostTask(
-      FROM_HERE,
-      base::Bind(&OAuthClient::Delegate::OnRefreshTokenResponse,
-                 base::Unretained(delegate_),
-                 access_token, expires_in_seconds));
-  // Queue a token exchange for 1 minute before this one expires.
-  message_loop_->PostDelayedTask(FROM_HERE,
-                                 base::Bind(&OAuthClient::RefreshToken,
-                                            base::Unretained(this)),
-                                 base::TimeDelta::FromSeconds(
-                                     expires_in_seconds - 60));
-}
-
-void OAuthClient::OnOAuthError() {
-  message_loop_->PostTask(
-      FROM_HERE,
-      base::Bind(&OAuthClient::Delegate::OnOAuthError,
-                 base::Unretained(delegate_)));
-}
-
-void OAuthClient::OnNetworkError(int response_code) {
-  // TODO(jamiewalch): Set a sensible retry limit and integrate with
-  // SignalingConnector to restart the reconnection process (crbug.com/118928).
-  NOTREACHED();
-}
-
-void OAuthClient::RefreshToken() {
-#ifdef OFFICIAL_BUILD
-  OAuthClientInfo client_info = {
-    "440925447803-avn2sj1kc099s0r7v62je5s339mu0am1.apps.googleusercontent.com",
-    "Bgur6DFiOMM1h8x-AQpuTQlK"
-  };
-#else  // OFFICIAL_BUILD
-  OAuthClientInfo client_info = {
-    "440925447803-2pi3v45bff6tp1rde2f7q6lgbor3o5uj.apps.googleusercontent.com",
-    "W2ieEsG-R1gIA4MMurGrgMc_"
-  };
-#endif  // !OFFICIAL_BUILD
-  gaia_oauth_client_->RefreshToken(client_info, refresh_token_, -1, this);
-}
-
-}  // namespace remoting
diff --git a/remoting/host/oauth_client.h b/remoting/host/oauth_client.h
deleted file mode 100644
index a92077a..0000000
--- a/remoting/host/oauth_client.h
+++ /dev/null
@@ -1,72 +0,0 @@
-// Copyright (c) 2012 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.
-//
-// This class implements the OAuth2 client for the Chromoting host.
-
-#ifndef REMOTING_HOST_OAUTH_CLIENT_H_
-#define REMOTING_HOST_OAUTH_CLIENT_H_
-
-#include <string>
-
-#include "base/memory/scoped_ptr.h"
-#include "remoting/host/gaia_oauth_client.h"
-
-namespace base {
-class MessageLoopProxy;
-}  // namespace base
-
-namespace remoting {
-
-class URLRequestContextGetter;
-
-class OAuthClient : public GaiaOAuthClient::Delegate {
- public:
-  class Delegate {
-   public:
-    virtual ~Delegate() { }
-    // Invoked on a successful response to the RefreshToken request.
-    virtual void OnRefreshTokenResponse(const std::string& access_token,
-                                        int expires_in_seconds) = 0;
-    // Invoked when there is an OAuth error with one of the requests.
-    virtual void OnOAuthError() = 0;
-  };
-
-  OAuthClient();
-  virtual ~OAuthClient();
-
-  // Notify the delegate on the speficied message loop when an access token is
-  // available. As long as the refresh is successful, another will be queued,
-  // timed just before the access token expires. As soon as an error occurs,
-  // this automatic refresh behaviour is cancelled.
-  //
-  // The delegate is accessed on the specified message loop, and must out-live
-  // it.
-  void Start(const scoped_refptr<URLRequestContextGetter>& url_context_,
-             const std::string& refresh_token,
-             Delegate* delegate,
-             base::MessageLoopProxy* message_loop);
-
-  // Overridden from GaiaOAuthClient::Delegate
-  virtual void OnGetTokensResponse(const std::string& refresh_token,
-                                   const std::string& access_token,
-                                   int expires_in_seconds) OVERRIDE;
-  virtual void OnRefreshTokenResponse(const std::string& access_token,
-                                      int expires) OVERRIDE;
-  virtual void OnOAuthError() OVERRIDE;
-  virtual void OnNetworkError(int response_code) OVERRIDE;
-
- private:
-  void RefreshToken();
-
-  scoped_ptr<GaiaOAuthClient> gaia_oauth_client_;
-  std::string refresh_token_;
-  OAuthClient::Delegate* delegate_;
-  scoped_refptr<base::MessageLoopProxy> message_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(OAuthClient);
-};
-
-}  // namespace remoting
-
-#endif  // REMOTING_HOST_OAUTH_CLIENT_H_
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index cc5438d..8ed3f32 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -37,7 +37,6 @@
 #include "remoting/host/host_event_logger.h"
 #include "remoting/host/json_host_config.h"
 #include "remoting/host/log_to_server.h"
-#include "remoting/host/oauth_client.h"
 #include "remoting/host/policy_hack/nat_policy.h"
 #include "remoting/host/signaling_connector.h"
 #include "remoting/jingle_glue/xmpp_signal_strategy.h"
@@ -73,8 +72,7 @@ const int kMaxPortNumber = 12409;
 namespace remoting {
 
 class HostProcess
-    : public OAuthClient::Delegate,
-      public HeartbeatSender::Listener {
+    : public HeartbeatSender::Listener {
  public:
   HostProcess()
       : message_loop_(MessageLoop::TYPE_UI),
@@ -110,10 +108,7 @@ class HostProcess
   }
 
   void ConfigUpdated() {
-    // The auth tokens can't be updated once the host is running, so we don't
-    // need to check the pending state, but it's a required parameter.
-    bool tokens_pending;
-    if (LoadConfig(&tokens_pending)) {
+    if (LoadConfig()) {
       context_->network_message_loop()->PostTask(
           FROM_HERE,
           base::Bind(&HostProcess::CreateAuthenticatorFactory,
@@ -169,23 +164,11 @@ class HostProcess
   }
 
   int Run() {
-    bool tokens_pending = false;
-    if (!LoadConfig(&tokens_pending)) {
+    if (!LoadConfig()) {
       return kInvalidHostConfigurationExitCode;
     }
-    if (tokens_pending) {
-      // If we have an OAuth refresh token, then XmppSignalStrategy can't
-      // handle it directly, so refresh it asynchronously. A task will be
-      // posted on the message loop to start watching the NAT policy when
-      // the access token is available.
-      //
-      // TODO(sergeyu): Move this code to SignalingConnector.
-      oauth_client_.Start(context_->url_request_context_getter(),
-                          oauth_refresh_token_, this,
-                          message_loop_.message_loop_proxy());
-    } else {
-      StartWatchingNatPolicy();
-    }
+
+    StartWatchingNatPolicy();
 
 #if defined(OS_MACOSX) || defined(OS_WIN)
     context_->file_message_loop()->PostTask(
@@ -198,29 +181,6 @@ class HostProcess
     return exit_code_;
   }
 
-  // Overridden from OAuthClient::Delegate
-  virtual void OnRefreshTokenResponse(const std::string& access_token,
-                                      int expires) OVERRIDE {
-    xmpp_auth_token_ = access_token;
-    // If there's already a signal strategy object, update it ready for the
-    // next time it calls Connect. If not, then this is the initial token
-    // exchange, so proceed to the next stage of connection.
-    if (signal_strategy_.get()) {
-      context_->network_message_loop()->PostTask(
-          FROM_HERE, base::Bind(
-              &XmppSignalStrategy::SetAuthInfo,
-              base::Unretained(signal_strategy_.get()),
-              xmpp_login_, xmpp_auth_token_, xmpp_auth_service_));
-    } else {
-      StartWatchingNatPolicy();
-    }
-  }
-
-  virtual void OnOAuthError() OVERRIDE {
-    LOG(ERROR) << "OAuth: invalid credentials.";
-    Shutdown(kInvalidOauthCredentialsExitCode);
-  }
-
   // Overridden from HeartbeatSender::Listener
   virtual void OnUnknownHostIdError() OVERRIDE {
     LOG(ERROR) << "Host ID not found.";
@@ -236,7 +196,7 @@ class HostProcess
   }
 
   // Read Host config from disk, returning true if successful.
-  bool LoadConfig(bool* tokens_pending) {
+  bool LoadConfig() {
     JsonHostConfig host_config(host_config_path_);
     JsonHostConfig auth_config(auth_config_path_);
 
@@ -280,8 +240,7 @@ class HostProcess
       return false;
     }
 
-    *tokens_pending = oauth_refresh_token_ != "";
-    if (*tokens_pending) {
+    if (!oauth_refresh_token_.empty()) {
       xmpp_auth_token_ = "";  // This will be set to the access token later.
       xmpp_auth_service_ = "oauth2";
     } else if (!auth_config.GetString(kXmppAuthServiceConfigPath,
@@ -324,8 +283,19 @@ class HostProcess
       signal_strategy_.reset(
           new XmppSignalStrategy(context_->jingle_thread(), xmpp_login_,
                                  xmpp_auth_token_, xmpp_auth_service_));
+
       signaling_connector_.reset(
           new SignalingConnector(signal_strategy_.get()));
+
+      if (!oauth_refresh_token_.empty()) {
+        scoped_ptr<SignalingConnector::OAuthCredentials> oauth_credentials(
+            new SignalingConnector::OAuthCredentials(
+                xmpp_login_, oauth_refresh_token_));
+        signaling_connector_->EnableOAuth(
+            oauth_credentials.Pass(),
+            base::Bind(&HostProcess::OnOAuthFailed, base::Unretained(this)),
+            context_->url_request_context_getter());
+      }
     }
 
     if (!desktop_environment_.get()) {
@@ -379,6 +349,10 @@ class HostProcess
     StartHost();
   }
 
+  void OnOAuthFailed() {
+    Shutdown(kInvalidOauthCredentialsExitCode);
+  }
+
   void Shutdown(int exit_code) {
     exit_code_ = exit_code;
     message_loop_.PostTask(FROM_HERE, MessageLoop::QuitClosure());
@@ -399,7 +373,6 @@ class HostProcess
   std::string xmpp_auth_service_;
 
   std::string oauth_refresh_token_;
-  OAuthClient oauth_client_;
 
   scoped_ptr<policy_hack::NatPolicy> nat_policy_;
   bool allow_nat_traversal_;
diff --git a/remoting/host/signaling_connector.cc b/remoting/host/signaling_connector.cc
index 6cac1c1..9b80a81 100644
--- a/remoting/host/signaling_connector.cc
+++ b/remoting/host/signaling_connector.cc
@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "remoting/host/chromoting_host_context.h"
+#include "remoting/host/url_request_context.h"
 
 namespace remoting {
 
@@ -15,11 +17,22 @@ namespace {
 // to the maximum specified here.
 const int kMaxReconnectDelaySeconds = 10 * 60;
 
+// Time when we we try to update OAuth token before its expiration.
+const int kTokenUpdateTimeBeforeExpirySeconds = 60;
+
 }  // namespace
 
-SignalingConnector::SignalingConnector(SignalStrategy* signal_strategy)
+SignalingConnector::OAuthCredentials::OAuthCredentials(
+    const std::string& login_value,
+    const std::string& refresh_token_value)
+    : login(login_value),
+      refresh_token(refresh_token_value) {
+}
+
+SignalingConnector::SignalingConnector(XmppSignalStrategy* signal_strategy)
     : signal_strategy_(signal_strategy),
-      reconnect_attempts_(0) {
+      reconnect_attempts_(0),
+      refreshing_oauth_token_(false) {
   net::NetworkChangeNotifier::AddOnlineStateObserver(this);
   net::NetworkChangeNotifier::AddIPAddressObserver(this);
   signal_strategy_->AddListener(this);
@@ -32,6 +45,15 @@ SignalingConnector::~SignalingConnector() {
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
 }
 
+void SignalingConnector::EnableOAuth(
+    scoped_ptr<OAuthCredentials> oauth_credentials,
+    const base::Closure& oauth_failed_callback,
+    URLRequestContextGetter* url_context) {
+  oauth_credentials_ = oauth_credentials.Pass();
+  oauth_failed_callback_ = oauth_failed_callback;
+  gaia_oauth_client_.reset(new GaiaOAuthClient(kGaiaOAuth2Url, url_context));
+}
+
 void SignalingConnector::OnSignalStrategyStateChange(
     SignalStrategy::State state) {
   DCHECK(CalledOnValidThread());
@@ -60,7 +82,48 @@ void SignalingConnector::OnOnlineStateChanged(bool online) {
   }
 }
 
+void SignalingConnector::OnGetTokensResponse(const std::string& refresh_token,
+                                             const std::string& access_token,
+                                             int expires_seconds) {
+  NOTREACHED();
+}
+
+void SignalingConnector::OnRefreshTokenResponse(const std::string& access_token,
+                                                int expires_seconds) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(oauth_credentials_.get());
+  LOG(INFO) << "Received OAuth token.";
+  refreshing_oauth_token_ = false;
+  auth_token_expiry_time_ = base::Time::Now() +
+      base::TimeDelta::FromSeconds(expires_seconds) -
+      base::TimeDelta::FromSeconds(kTokenUpdateTimeBeforeExpirySeconds);
+  signal_strategy_->SetAuthInfo(oauth_credentials_->login,
+                                access_token, "oauth2");
+
+  // Now that we've got the new token, try to connect using it.
+  DCHECK_EQ(signal_strategy_->GetState(), SignalStrategy::DISCONNECTED);
+  signal_strategy_->Connect();
+}
+
+void SignalingConnector::OnOAuthError() {
+  DCHECK(CalledOnValidThread());
+  LOG(ERROR) << "OAuth: invalid credentials.";
+  refreshing_oauth_token_ = false;
+  reconnect_attempts_++;
+  oauth_failed_callback_.Run();
+}
+
+void SignalingConnector::OnNetworkError(int response_code) {
+  DCHECK(CalledOnValidThread());
+  LOG(ERROR) << "Network error when trying to update OAuth token: "
+             << response_code;
+  refreshing_oauth_token_ = false;
+  reconnect_attempts_++;
+  ScheduleTryReconnect();
+}
+
 void SignalingConnector::ScheduleTryReconnect() {
+  DCHECK(CalledOnValidThread());
   if (timer_.IsRunning() || net::NetworkChangeNotifier::IsOffline())
     return;
   int delay_s = std::min(1 << (reconnect_attempts_ * 2),
@@ -70,6 +133,7 @@ void SignalingConnector::ScheduleTryReconnect() {
 }
 
 void SignalingConnector::ResetAndTryReconnect() {
+  DCHECK(CalledOnValidThread());
   signal_strategy_->Disconnect();
   reconnect_attempts_ = 0;
   timer_.Stop();
@@ -79,9 +143,36 @@ void SignalingConnector::ResetAndTryReconnect() {
 void SignalingConnector::TryReconnect() {
   DCHECK(CalledOnValidThread());
   if (signal_strategy_->GetState() == SignalStrategy::DISCONNECTED) {
-    LOG(INFO) << "Attempting to connect signaling.";
-    signal_strategy_->Connect();
+    bool need_new_auth_token = oauth_credentials_.get() &&
+        (auth_token_expiry_time_.is_null() ||
+         base::Time::Now() >= auth_token_expiry_time_);
+    if (need_new_auth_token) {
+      RefreshOAuthToken();
+    } else {
+      LOG(INFO) << "Attempting to connect signaling.";
+      signal_strategy_->Connect();
+    }
   }
 }
 
+void SignalingConnector::RefreshOAuthToken() {
+  DCHECK(CalledOnValidThread());
+  LOG(INFO) << "Refreshing OAuth token.";
+  DCHECK(!refreshing_oauth_token_);
+#ifdef OFFICIAL_BUILD
+  OAuthClientInfo client_info = {
+    "440925447803-avn2sj1kc099s0r7v62je5s339mu0am1.apps.googleusercontent.com",
+    "Bgur6DFiOMM1h8x-AQpuTQlK"
+  };
+#else  // OFFICIAL_BUILD
+  OAuthClientInfo client_info = {
+    "440925447803-2pi3v45bff6tp1rde2f7q6lgbor3o5uj.apps.googleusercontent.com",
+    "W2ieEsG-R1gIA4MMurGrgMc_"
+  };
+#endif  // !OFFICIAL_BUILD
+  refreshing_oauth_token_ = true;
+  gaia_oauth_client_->RefreshToken(
+      client_info, oauth_credentials_->refresh_token, 1, this);
+}
+
 }  // namespace remoting
diff --git a/remoting/host/signaling_connector.h b/remoting/host/signaling_connector.h
index 23bb667..227bf71 100644
--- a/remoting/host/signaling_connector.h
+++ b/remoting/host/signaling_connector.h
@@ -10,10 +10,13 @@
 #include "base/threading/non_thread_safe.h"
 #include "base/timer.h"
 #include "net/base/network_change_notifier.h"
-#include "remoting/jingle_glue/signal_strategy.h"
+#include "remoting/host/gaia_oauth_client.h"
+#include "remoting/jingle_glue/xmpp_signal_strategy.h"
 
 namespace remoting {
 
+class URLRequestContextGetter;
+
 // SignalingConnector listens for SignalStrategy status notifications
 // and attempts to keep it connected when possible. When signalling is
 // not connected it keeps trying to reconnect it until it is
@@ -25,11 +28,25 @@ class SignalingConnector
       public base::NonThreadSafe,
       public SignalStrategy::Listener,
       public net::NetworkChangeNotifier::OnlineStateObserver,
-      public net::NetworkChangeNotifier::IPAddressObserver {
+      public net::NetworkChangeNotifier::IPAddressObserver,
+      public GaiaOAuthClient::Delegate {
  public:
-  SignalingConnector(SignalStrategy* signal_strategy);
+  struct OAuthCredentials {
+    OAuthCredentials(const std::string& login_value,
+                     const std::string& refresh_token_value);
+    std::string login;
+    std::string refresh_token;
+  };
+
+  // OAuth token is updated refreshed when |oauth_credentials| is
+  // not NULL.
+  SignalingConnector(XmppSignalStrategy* signal_strategy);
   virtual ~SignalingConnector();
 
+  void EnableOAuth(scoped_ptr<OAuthCredentials> oauth_credentials,
+                   const base::Closure& oauth_failed_callback,
+                   URLRequestContextGetter* url_context);
+
   // SignalStrategy::Listener interface.
   virtual void OnSignalStrategyStateChange(
       SignalStrategy::State state) OVERRIDE;
@@ -40,16 +57,34 @@ class SignalingConnector
   // NetworkChangeNotifier::OnlineStateObserver interface.
   virtual void OnOnlineStateChanged(bool online) OVERRIDE;
 
+  // GaiaOAuthClient::Delegate interface.
+  virtual void OnGetTokensResponse(const std::string& refresh_token,
+                                   const std::string& access_token,
+                                   int expires_seconds) OVERRIDE;
+  virtual void OnRefreshTokenResponse(const std::string& access_token,
+                                      int expires_seconds) OVERRIDE;
+  virtual void OnOAuthError() OVERRIDE;
+  virtual void OnNetworkError(int response_code) OVERRIDE;
+
  private:
   void ScheduleTryReconnect();
   void ResetAndTryReconnect();
   void TryReconnect();
 
-  SignalStrategy* signal_strategy_;
+  void RefreshOAuthToken();
+
+  XmppSignalStrategy* signal_strategy_;
+
+  scoped_ptr<OAuthCredentials> oauth_credentials_;
+  base::Closure oauth_failed_callback_;
+  scoped_ptr<GaiaOAuthClient> gaia_oauth_client_;
 
   // Number of times we tried to connect without success.
   int reconnect_attempts_;
 
+  bool refreshing_oauth_token_;
+  base::Time auth_token_expiry_time_;
+
   base::OneShotTimer<SignalingConnector> timer_;
 
   DISALLOW_COPY_AND_ASSIGN(SignalingConnector);
diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc
index eccc04b..7d4c3f7 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -287,7 +287,7 @@ class SimpleHost : public HeartbeatSender::Listener {
   std::string xmpp_auth_token_;
   std::string xmpp_auth_service_;
 
-  scoped_ptr<SignalStrategy> signal_strategy_;
+  scoped_ptr<XmppSignalStrategy> signal_strategy_;
   scoped_ptr<SignalingConnector> signaling_connector_;
   scoped_ptr<DesktopEnvironment> desktop_environment_;
   scoped_ptr<LogToServer> log_to_server_;
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index 7a274d2..80c6c5d 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -828,6 +828,8 @@
         'remoting_protocol',
         'differ_block',
         '../crypto/crypto.gyp:crypto',
+        # TODO(hclam): Remove this dependency once we don't use URLFetcher.
+        '../content/content.gyp:content_common',
       ],
       'sources': [
         'host/capturer.h',
@@ -895,8 +897,6 @@
         'host/local_input_monitor_win.cc',
         'host/log_to_server.cc',
         'host/log_to_server.h',
-        'host/oauth_client.cc',
-        'host/oauth_client.h',
         'host/policy_hack/nat_policy.h',
         'host/policy_hack/nat_policy.cc',
         'host/policy_hack/nat_policy_linux.cc',
@@ -1055,8 +1055,6 @@
         '../base/base.gyp:base',
         '../base/base.gyp:base_i18n',
         '../media/media.gyp:media',
-        # TODO(hclam): Remove this dependency once we don't use URLFetcher.
-        '../content/content.gyp:content_common',
       ],
       'sources': [
         'host/branding.cc',