Adding the AccessTokenFetcher and Environment class to the app remoting test

remoting/test/app_remoting_test_driver_environment.cc

+// 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/app_remoting_test_driver_environment.h"
+
+#include "base/bind.h"
+#include "base/callback_forward.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+
+namespace remoting {
+namespace test {
+
+AppRemotingTestDriverEnvironment* AppRemotingSharedData;
+
+AppRemotingTestDriverEnvironment::AppRemotingTestDriverEnvironment(
+    const std::string& user_name,
+    const std::string& service_environment) :
+    user_name_(user_name),
+    service_environment_(service_environment),
+    initialized_(false) {
+  DCHECK(!user_name_.empty());
+  DCHECK(!service_environment.empty());
+}
+
+AppRemotingTestDriverEnvironment::~AppRemotingTestDriverEnvironment() {
+}
+
+bool AppRemotingTestDriverEnvironment::Initialize(
+    const std::string& auth_code) {
+  if (initialized_) {

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

Do you want to let things try to initialize this several times?

What if one test initializes it with one auth-code and a different test uses a
different one?

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

The Environment class is only initialized once (before tests run).  We can
assume that having an access token means it has been initialized (I removed the
bool per you comment in the header file).

+    return true;
+  }
+
+  if (!refresh_token_storage_.get()) {

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

Drop .get()

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

Done.

+    refresh_token_storage_.reset(new remoting::test::RefreshTokenStorage());
+  }
+
+  // Check to see if we have a refresh token stored for this user.
+  refresh_token_ = refresh_token_storage_->ReadRefreshTokenFromDisk(user_name_);
+  if (refresh_token_.empty()) {
+    // This isn't necessarily an error as this might be a first run scenario.
+    DVLOG(1) << "No refresh token stored for " << user_name_;
+
+    if (auth_code.empty()) {
+      // No token and no Auth code means no service connectivity, bail!
+      LOG(ERROR) << "Cannot retrieve an access token without a stored refresh"
+                 << " token on disk or an auth_code passed into the tool";
+      return false;
+    }
+  }
+
+  if (!RetrieveAccessToken(auth_code)) {
+    // If we cannot retrieve an access token, then nothing is going to work and
+    // we should let the caller know that our object is not ready to be used.
+    return false;
+  }
+
+  initialized_ = true;
+
+  return true;
+}
+
+bool AppRemotingTestDriverEnvironment::RefreshAccessToken() {
+  DCHECK(!refresh_token_.empty());
+
+  std::string auth_code;  // Empty auth code is used when refreshing.

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

?

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

Done.

+  return RetrieveAccessToken(auth_code);

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

RetrieveAccessToken might be better being RefreshAccessTokenFromAuthCode() -
then it fits better w/ this function's name. With your comment in place you can
just pass std::string() directly to the call.

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

Done.  I had thought about splitting the RetrieveAccessToken function up into
auth code and refresh token specific functions, but there would be a lot of code
duplication in that case.  I'd like to leave it as-is and update the
comment/param passed.

[Wez, 2015/02/19 22:00:22]

Acknowledged.

+}
+
+void AppRemotingTestDriverEnvironment::SetAccessTokenFetcherForTesting(
+    remoting::test::AccessTokenFetcher* mock_fetcher) {

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

This parameter should be a scoped_ptr<>

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

Done.

+  access_token_fetcher_.reset(mock_fetcher);
+}
+
+void AppRemotingTestDriverEnvironment::SetRefreshTokenStorageForTesting(
+    remoting::test::RefreshTokenStorageInterface* mock_refresh_token_storage) {

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

As above

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

Done.

+  refresh_token_storage_.reset(mock_refresh_token_storage);
+}
+
+void AppRemotingTestDriverEnvironment::SetUp() {
+  // The object should already be initialized by the time SetUp is called.

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

I thought SetUp and TearDown were called once per-test...?

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

The environment class is setup and torn down once per test suite lifetime, SetUp
and TearDown are called more or less frequently depending on whether you are
talking about an Environment class, Static Test Fixture Method, or Test Fixture
Method.

[Wez, 2015/02/19 22:00:22]

Acknowledged.

+  DCHECK(initialized_);
+}
+
+void AppRemotingTestDriverEnvironment::TearDown() {
+  if (!initialized_) {

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

Why would TearDown be called if we're not initialized? Surely that's a bug in
the framework?

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

Done.

+    return;
+  }
+
+  // Tear down the members that we created in the SetUp method here.
+
+  initialized_ = false;
+}
+
+bool AppRemotingTestDriverEnvironment::RetrieveAccessToken(
+    const std::string& auth_code) {
+  scoped_ptr<base::MessageLoopForIO> message_loop;
+
+  if (!base::MessageLoop::current()) {
+    // Create a temporary message loop if the current thread does not already
+    // have one so we can use its task runner for our network request.
+    message_loop.reset(new base::MessageLoopForIO);
+  }
+  DCHECK(!network_request_run_loop_.get());
+  network_request_run_loop_.reset(new base::RunLoop());

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

You can just create this on the stack, can't you, since you only use the RunLoop
in this function's scope and Bind() the RunLoop::QuitClosure() into
OnAccessTokenRetrieved to call?

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

Done.

+
+  access_token_.clear();
+
+  AccessTokenCallback access_token_callback = base::Bind(
+      &AppRemotingTestDriverEnvironment::OnAccessTokenRetrieved,
+      base::Unretained(this));
+
+  if (!access_token_fetcher_.get()) {
+    access_token_fetcher_.reset(new remoting::test::AccessTokenFetcher());

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

Are you sure that the fetcher won't bind to the potentially-temporary
MessageLoop in some way?

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

I updated the code so the real access token fetcher is released at the end of
the function, however a mock class is not released which is what I want for the
unit tests.

[Wez, 2015/02/19 22:00:22]

Sorry, I don't understand what you mean.

+  }
+
+  if (!auth_code.empty()) {
+    // If the user passed in an authcode, then use it to retrieve an
+    // updated access/refresh token.
+    access_token_fetcher_->GetAccessTokenFromAuthCode(auth_code,
+        access_token_callback);
+  } else {
+    DCHECK(!refresh_token_.empty());
+
+    access_token_fetcher_->GetAccessTokenFromRefreshToken(refresh_token_,
+      access_token_callback);
+  }
+
+  network_request_run_loop_->Run();
+  network_request_run_loop_.reset();
+
+  // If we were using an auth_code and received a valid refresh token,
+  // then we want to store it locally.  If we had an auth code and did not
+  // receive a refresh token, then we should let the user know and exit.
+  if (!auth_code.empty()) {
+    if (!refresh_token_.empty()) {
+      if (!refresh_token_storage_->
+          WriteRefreshTokenToDisk(user_name_, refresh_token_)) {
+        // If we failed to persist the refresh token, then we should let the
+        // user sort out the issue before continuing.
+        return false;
+      }
+    } else {
+      LOG(ERROR) << "Failed to use AUTH CODE to retrieve a refresh token.\n"
+                 << "Was the one-time use AUTH CODE used more than once?";
+      return false;
+    }
+  }
+
+  if (access_token_.empty()) {
+    LOG(ERROR) << "Failed to retrieve access token.";
+    return false;
+  }
+
+  return true;
+}
+
+void AppRemotingTestDriverEnvironment::OnAccessTokenRetrieved(
+    const std::string& access_token,
+    const std::string& refresh_token) {
+  access_token_ = access_token;
+  refresh_token_ = refresh_token;
+
+  network_request_run_loop_->Quit();
+}
+
+}  // namespace test
+}  // namespace remoting