Adding the AccessTokenFetcher and Environment class to the app remoting test

remoting/test/access_token_fetcher_unittest.cc

Index: remoting/test/access_token_fetcher_unittest.cc
diff --git a/remoting/test/access_token_fetcher_unittest.cc b/remoting/test/access_token_fetcher_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..270c8ae390c605bacd616fd2b497fa9362685332
--- /dev/null
+++ b/remoting/test/access_token_fetcher_unittest.cc
@@ -0,0 +1,537 @@
+// Copyright 2015 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/test/access_token_fetcher.h"
+
+#include "base/bind.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+  const char kAuthCodeValue[] = "test_auth_code_value";

[Wez, 2015/02/13 03:01:52]

nit: Indentation

[joedow, 2015/02/14 02:31:26]

Done.

+  const char kAccessTokenValue[] = "test_access_token_value";
+  const char kRefreshTokenValue[] = "test_refresh_token_value";
+  const int kTokenExpirationValue = 3600;
+  const int kNetworkErrorCode = 404;
+}
+
+namespace remoting {
+namespace test {
+
+using testing::_;
+
+// This class implements the adapter interface which is used by the
+// AccessTokenFetcher to make its gaia network calls.
+class MockGaiaOAuthClient : public GaiaOAuthClientAdapter {
+ public:
+  MockGaiaOAuthClient() : GaiaOAuthClientAdapter(nullptr) {}
+  ~MockGaiaOAuthClient() override {}
+
+  void OnAccessTokenRetrieved(

[Wez, 2015/02/13 03:01:52]

Style-guide prefers that non-accessor methods not be implemented in-line.

[joedow, 2015/02/14 02:31:27]

Done.

+      const std::string& access_token,
+      const std::string& refresh_token) {
+    access_token_retrieved = access_token;
+    refresh_token_retrieved = refresh_token;
+  }
+
+  // Since we only want the delegate parameter for this mock, we will override
+  // the virtual members and have them call simplified versions.

[Wez, 2015/02/13 03:01:52]

Since you'll ignore the other three parameters, doesn't that just save you _,_,_
from the EXPECT_CALL()s? Is it worth wrapping things just for that?

[joedow, 2015/02/14 02:31:27]

Done.

+  void GetTokensFromAuthCode(
+      const gaia::OAuthClientInfo& oauth_client_info,
+      const std::string& auth_code,
+      int max_retries,
+      gaia::GaiaOAuthClient::Delegate* delegate) override {
+    GetTokensFromAuthCodeImpl(delegate);
+  }
+  void RefreshToken(
+      const gaia::OAuthClientInfo& oauth_client_info,
+      const std::string& refresh_token,
+      const std::vector<std::string>& scopes,
+      int max_retries,
+      gaia::GaiaOAuthClient::Delegate* delegate) override {
+    RefreshTokenImpl(delegate);
+  }
+  void GetTokenInfo(
+      const std::string& oauth_access_token,
+      int max_retries,
+      gaia::GaiaOAuthClient::Delegate* delegate) override {
+    GetTokenInfoImpl(delegate);
+  }
+
+  MOCK_METHOD1(GetTokensFromAuthCodeImpl,
+      void(gaia::GaiaOAuthClient::Delegate* delegate));
+
+  MOCK_METHOD1(RefreshTokenImpl,
+      void(gaia::GaiaOAuthClient::Delegate* delegate));
+
+  MOCK_METHOD1(GetTokenInfoImpl,
+      void(gaia::GaiaOAuthClient::Delegate* delegate));
+
+  void DelegateToCompleteCallbacks() {

[Wez, 2015/02/13 03:01:51]

This function name isn't very descriptive - looks like this is called to set
things up to mock a success? Or is the issue that you want to pull the delegate
out of the mocked methods to pass to the completion methods below? Is there no
cleaner way to do that?

This class seems to have ended up being a hybrid mock/fake; would it be cleaner
if it were a pure fake, i.e. the behaviour below is coded in rather than using
gmock, and you then provide explicit call-count accessors for the tests to use
to verify things?

[joedow, 2015/02/14 02:31:26]

Done.

+    ON_CALL(*this, GetTokensFromAuthCodeImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::CompleteAuthCodeCallback));
+    ON_CALL(*this, RefreshTokenImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::CompleteRefreshCallback));
+    ON_CALL(*this, GetTokenInfoImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::CompleteTokenInfoCallback));
+  }
+
+  void DelegateToOAuthErrorCallback() {
+    ON_CALL(*this, GetTokensFromAuthCodeImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::OAuthErrorCallback));
+    ON_CALL(*this, RefreshTokenImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::OAuthErrorCallback));
+    ON_CALL(*this, GetTokenInfoImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::OAuthErrorCallback));
+  }
+
+  void DelegateToNetworkErrorCallback() {
+    ON_CALL(*this, GetTokensFromAuthCodeImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::NetworkErrorCallback));
+    ON_CALL(*this, RefreshTokenImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::NetworkErrorCallback));
+    ON_CALL(*this, GetTokenInfoImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::NetworkErrorCallback));
+  }
+
+  void DelegateToInvalidTokenCallback() {
+    ON_CALL(*this, GetTokensFromAuthCodeImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::CompleteAuthCodeCallback));
+    ON_CALL(*this, RefreshTokenImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::CompleteRefreshCallback));
+    ON_CALL(*this, GetTokenInfoImpl(_))
+        .WillByDefault(testing::Invoke(this,
+            &MockGaiaOAuthClient::InvalidTokenInfoCallback));
+  }
+
+  void CompleteAuthCodeCallback(gaia::GaiaOAuthClient::Delegate* delegate) {
+    DCHECK(delegate);
+
+    delegate->OnGetTokensResponse(
+      kRefreshTokenValue,
+      kAccessTokenValue,
+      kTokenExpirationValue);
+  }
+
+  void CompleteRefreshCallback(gaia::GaiaOAuthClient::Delegate* delegate) {
+    DCHECK(delegate);
+
+    delegate->OnRefreshTokenResponse(kAccessTokenValue, kTokenExpirationValue);
+  }
+
+  void CompleteTokenInfoCallback(gaia::GaiaOAuthClient::Delegate* delegate) {
+    DCHECK(delegate);
+
+    // This call needs a dictionary object, but does not it to be populated.
+    scoped_ptr<base::DictionaryValue> token_info(new base::DictionaryValue());
+    delegate->OnGetTokenInfoResponse(token_info.Pass());

[Wez, 2015/02/13 03:01:52]

nit: Just use make_scoped_ptr(new base::DictionaryValue()) here, and avoid the
local variable?

[joedow, 2015/02/14 02:31:27]

Acknowledged.

+  }
+
+  void InvalidTokenInfoCallback(gaia::GaiaOAuthClient::Delegate* delegate) {
+    DCHECK(delegate);
+
+    scoped_ptr<base::DictionaryValue> token_info(new base::DictionaryValue());

[Wez, 2015/02/13 03:01:52]

And here?

[joedow, 2015/02/14 02:31:27]

Acknowledged.

+    token_info->SetString("error", "invalid");
+    token_info->SetString("error_description", "Whoa! Invalid Token!");
+    delegate->OnGetTokenInfoResponse(token_info.Pass());
+  }
+
+  void OAuthErrorCallback(gaia::GaiaOAuthClient::Delegate* delegate) {
+    DCHECK(delegate);
+
+    delegate->OnOAuthError();
+  }
+
+  void NetworkErrorCallback(gaia::GaiaOAuthClient::Delegate* delegate) {
+    DCHECK(delegate);
+
+    delegate->OnNetworkError(kNetworkErrorCode);
+  }
+
+    // Verification members used to observe the value of the data retrieved.

[Wez, 2015/02/13 03:01:52]

nit: Indentation

[joedow, 2015/02/14 02:31:27]

Acknowledged.

+  std::string access_token_retrieved;
+  std::string refresh_token_retrieved;
+};

[Wez, 2015/02/13 03:01:51]

DISALLOW_COPY_AND_ASSIGN()?

[joedow, 2015/02/14 02:31:26]

Acknowledged.

+
+// This class uses all of the same logic as the normal AccessTokenFetcher but it
+// overrides the function which refreshes the GaiaOAuthClient so it can use a
+// mock instance.
+class TestAccessTokenFetcher : public AccessTokenFetcher {

[Wez, 2015/02/13 03:01:51]

nit: AccessTokenFetcherForTest or FakeAccessTokenFetcher or
MockAccessTokenFetcher?

[joedow, 2015/02/14 02:31:26]

Done.

+ public:
+  explicit TestAccessTokenFetcher(GaiaOAuthClientAdapter* mock_client) {

[Wez, 2015/02/13 03:01:51]

Make this a scoped_ptr and use make_scoped_ptr() in calls?

[joedow, 2015/02/14 02:31:26]

Done.

+    DCHECK(mock_client);
+    auth_client_.reset(mock_client);
+  }
+  ~TestAccessTokenFetcher() override {}
+
+  void RefreshGaiaOAuthClientInstance() override {

[Wez, 2015/02/13 03:01:52]

If you follow the suggestion re CreateGaiaOAuthClient in the AccessTokenFetcher
then you'd need to provide an AddMockGaiaOAuthClient() method and populate a
list<> in this class, then have the Create...() method pop and return them in
turn, I think.

[joedow, 2015/02/14 02:31:26]

I don't think I need too make this complex (e.g. a list of objects).  The idea
is that the base class calls this function before every gaia call it makes and
when I am using the mock object, I do not want to release/new another instance. 
I can override the function and do nothing and everything works great.  I've
updated the comment so it is clearer about what I am doing.

+    // No need to refresh the object here since we use the same mock instance.
+  }
+};

[Wez, 2015/02/13 03:01:52]

DISALLOW_COPY_AND_ASSIGN

[joedow, 2015/02/14 02:31:26]

Done.

+
+TEST(AccessTokenFetcherTest, ExchangeAuthCodeForAccessToken) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToCompleteCallbacks();

[Wez, 2015/02/13 03:01:51]

Based on this usage, it looks like this should be called
SetSuccessfulCompletionMocks() or similar?

[joedow, 2015/02/14 02:31:26]

Acknowledged.

+
+  {
+    testing::Sequence call_sequence;
+
+    EXPECT_CALL(*mock_gaia_client,
+        GetTokensFromAuthCodeImpl(_)).
+        Times(1);
+
+    EXPECT_CALL(*mock_gaia_client,
+        GetTokenInfoImpl(_)).
+        Times(1);
+  }
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(0);
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.

[Wez, 2015/02/13 03:01:52]

nit: destroyed :)

[joedow, 2015/02/14 02:31:26]

Done.

+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  access_token_fetcher.GetAccessTokenFromAuthCode(
+      kAuthCodeValue,
+      access_token_callback);
+
+  EXPECT_STREQ(kAccessTokenValue,
+               mock_gaia_client->access_token_retrieved.c_str());
+  EXPECT_STREQ(kRefreshTokenValue,
+               mock_gaia_client->refresh_token_retrieved.c_str());
+}
+
+TEST(AccessTokenFetcherTest, ExchangeRefreshTokenForAccessToken) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToCompleteCallbacks();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  {
+    testing::Sequence call_sequence;
+
+    EXPECT_CALL(*mock_gaia_client,
+        RefreshTokenImpl(_)).
+        Times(1);
+
+    EXPECT_CALL(*mock_gaia_client,
+        GetTokenInfoImpl(_)).
+        Times(1);
+  }
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(0);
+
+  access_token_fetcher.GetAccessTokenFromRefreshToken(
+      kRefreshTokenValue,
+      access_token_callback);
+
+  EXPECT_STREQ(kAccessTokenValue,
+               mock_gaia_client->access_token_retrieved.c_str());
+  EXPECT_STREQ(kRefreshTokenValue,
+               mock_gaia_client->refresh_token_retrieved.c_str());
+}
+
+TEST(AccessTokenFetcherTest, MultipleAccessTokenCalls) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToCompleteCallbacks();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(2);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(3);
+
+  access_token_fetcher.GetAccessTokenFromAuthCode(
+      kAuthCodeValue,
+      access_token_callback);
+
+  EXPECT_STREQ(kAccessTokenValue,
+               mock_gaia_client->access_token_retrieved.c_str());
+  EXPECT_STREQ(kRefreshTokenValue,
+               mock_gaia_client->refresh_token_retrieved.c_str());
+
+  // Reset our token data for the next iteration.
+  mock_gaia_client->access_token_retrieved.clear();
+  mock_gaia_client->refresh_token_retrieved.clear();
+
+  access_token_fetcher.GetAccessTokenFromRefreshToken(
+      kRefreshTokenValue,
+      access_token_callback);
+
+  EXPECT_STREQ(kAccessTokenValue,
+               mock_gaia_client->access_token_retrieved.c_str());
+  EXPECT_STREQ(kRefreshTokenValue,
+               mock_gaia_client->refresh_token_retrieved.c_str());
+
+  // Reset our token data for the next iteration.
+  mock_gaia_client->access_token_retrieved.clear();
+  mock_gaia_client->refresh_token_retrieved.clear();
+
+  access_token_fetcher.GetAccessTokenFromRefreshToken(
+      kRefreshTokenValue,
+      access_token_callback);
+
+  EXPECT_STREQ(kAccessTokenValue,
+               mock_gaia_client->access_token_retrieved.c_str());
+  EXPECT_STREQ(kRefreshTokenValue,
+               mock_gaia_client->refresh_token_retrieved.c_str());
+}
+
+TEST(AccessTokenFetcherTest, ExchangeAuthCode_OAuthError) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToOAuthErrorCallback();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(0);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(0);
+
+  access_token_fetcher.GetAccessTokenFromAuthCode(
+      kAuthCodeValue,
+      access_token_callback);
+
+  // Our callback should have been called with empty strings.
+  EXPECT_STREQ(mock_gaia_client->access_token_retrieved.c_str(), "");
+  EXPECT_STREQ(mock_gaia_client->refresh_token_retrieved.c_str(), "");
+}
+
+TEST(AccessTokenFetcherTest, ExchangeRefreshToken_OAuthError) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToOAuthErrorCallback();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(0);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(0);
+
+  access_token_fetcher.GetAccessTokenFromRefreshToken(
+      kRefreshTokenValue,
+      access_token_callback);
+
+  // Our callback should have been called with empty strings.
+  EXPECT_STREQ(mock_gaia_client->access_token_retrieved.c_str(), "");
+  EXPECT_STREQ(mock_gaia_client->refresh_token_retrieved.c_str(), "");
+}
+
+TEST(AccessTokenFetcherTest, ExchangeAuthCode_NetworkError) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToNetworkErrorCallback();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(0);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(0);
+
+  access_token_fetcher.GetAccessTokenFromAuthCode(
+      kAuthCodeValue,
+      access_token_callback);
+
+  // Our callback should have been called with empty strings.
+  EXPECT_STREQ(mock_gaia_client->access_token_retrieved.c_str(), "");
+  EXPECT_STREQ(mock_gaia_client->refresh_token_retrieved.c_str(), "");
+}
+
+TEST(AccessTokenFetcherTest, ExchangeRefreshToken_NetworkError) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToNetworkErrorCallback();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(0);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(0);
+
+  access_token_fetcher.GetAccessTokenFromRefreshToken(
+      kRefreshTokenValue,
+      access_token_callback);
+
+  // Our callback should have been called with empty strings.
+  EXPECT_STREQ(mock_gaia_client->access_token_retrieved.c_str(), "");
+  EXPECT_STREQ(mock_gaia_client->refresh_token_retrieved.c_str(), "");
+}
+
+TEST(AccessTokenFetcherTest, AuthCode_GetTokenInfoResponse_InvalidToken) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToInvalidTokenCallback();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.
+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(0);
+
+  access_token_fetcher.GetAccessTokenFromAuthCode(
+      kAuthCodeValue,
+      access_token_callback);
+
+  // Our callback should have been called with empty strings.
+  EXPECT_STREQ(mock_gaia_client->access_token_retrieved.c_str(), "");
+  EXPECT_STREQ(mock_gaia_client->refresh_token_retrieved.c_str(), "");
+}
+
+TEST(AccessTokenFetcherTest, RefreshToken_GetTokenInfoResponse_InvalidToken) {
+  MockGaiaOAuthClient* mock_gaia_client = new MockGaiaOAuthClient();
+
+  mock_gaia_client->DelegateToInvalidTokenCallback();
+
+  // Passing the pointer transfers ownership to the token fetcher class.  We
+  // will continue to use the object as it will be alive until the token fetcher
+  // is destructed, but we do not want to free it at the end of the function.

[Wez, 2015/02/13 03:01:52]

Suggest moving this comment in this and the preceding tests, to the
|mock_gaia_client| declaration, and stating that you're using a bare pointer so
you can pass it off to the TestAccessTokenFetcher to own, while retaining a
reference to it for mocking purposes.

[joedow, 2015/02/14 02:31:26]

Done.

+  TestAccessTokenFetcher access_token_fetcher(mock_gaia_client);
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &MockGaiaOAuthClient::OnAccessTokenRetrieved,
+      base::Unretained(mock_gaia_client));
+
+  EXPECT_CALL(*mock_gaia_client,
+      RefreshTokenImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokenInfoImpl(_)).
+      Times(1);
+
+  EXPECT_CALL(*mock_gaia_client,
+      GetTokensFromAuthCodeImpl(_)).
+      Times(0);

[Wez, 2015/02/13 03:01:51]

Here and in earlier tests; can't you set these expectations the moment you
create the |mock_gaia_client|?

[joedow, 2015/02/14 02:31:26]

Done.

+
+  access_token_fetcher.GetAccessTokenFromRefreshToken(
+      kRefreshTokenValue,
+      access_token_callback);
+
+  // Our callback should have been called with empty strings.
+  EXPECT_STREQ(mock_gaia_client->access_token_retrieved.c_str(), "");
+  EXPECT_STREQ(mock_gaia_client->refresh_token_retrieved.c_str(), "");
+}
+
+}  // namespace test
+}  // namespace remoting