[Autofill Assistant] Extracted common ServiceRequestSender interface.

This CL splits the ServiceRequestSender into interface and
implementation. The immediate use case is to allow integration tests to
create their own java-driven instance.

Bug: b/171776026
Change-Id: I32b914afc2807949354727c5741d0bbc8b184cbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524529
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Marian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#826258}

chrome/browser/android/autofill_assistant/lite_service_bridge.cc

@@ -11,6 +11,7 @@
 #include "components/autofill_assistant/browser/service/api_key_fetcher.h"
 #include "components/autofill_assistant/browser/service/lite_service.h"
 #include "components/autofill_assistant/browser/service/server_url_fetcher.h"
+#include "components/autofill_assistant/browser/service/service_request_sender_impl.h"
 #include "content/public/browser/web_contents.h"
 
 namespace autofill_assistant {
@@ -48,7 +49,7 @@ jlong JNI_AutofillAssistantLiteService_CreateLiteService(
 
   ServerUrlFetcher url_fetcher{ServerUrlFetcher::GetDefaultServerUrl()};
   return reinterpret_cast<jlong>(new LiteService(
-      std::make_unique<ServiceRequestSender>(
+      std::make_unique<ServiceRequestSenderImpl>(
           web_contents->GetBrowserContext(),
           /* access_token_fetcher = */ nullptr,
           std::make_unique<NativeURLLoaderFactory>(),

chrome/browser/android/autofill_assistant/trigger_script_bridge_android.cc

@@ -12,7 +12,7 @@
 #include "chrome/common/channel_info.h"
 #include "components/autofill_assistant/browser/service/api_key_fetcher.h"
 #include "components/autofill_assistant/browser/service/server_url_fetcher.h"
-#include "components/autofill_assistant/browser/service/service_request_sender.h"
+#include "components/autofill_assistant/browser/service/service_request_sender_impl.h"
 #include "components/autofill_assistant/browser/service/simple_url_loader_factory.h"
 #include "components/autofill_assistant/browser/trigger_scripts/dynamic_trigger_conditions.h"
 #include "components/autofill_assistant/browser/trigger_scripts/static_trigger_conditions.h"
@@ -46,7 +46,7 @@ void TriggerScriptBridgeAndroid::StartTriggerScript(
       client,
       WebController::CreateForWebContents(client->GetWebContents(),
                                           &client_settings_),
-      std::make_unique<ServiceRequestSender>(
+      std::make_unique<ServiceRequestSenderImpl>(
           client->GetWebContents()->GetBrowserContext(),
           /* access_token_fetcher = */ nullptr,
           std::make_unique<NativeURLLoaderFactory>(),

components/autofill_assistant/browser/BUILD.gn

@@ -172,6 +172,8 @@ static_library("browser") {
     "service/service_impl.h",
     "service/service_request_sender.cc",
     "service/service_request_sender.h",
+    "service/service_request_sender_impl.cc",
+    "service/service_request_sender_impl.h",
     "service/simple_url_loader_factory.cc",
     "service/simple_url_loader_factory.h",
     "state.h",
@@ -355,7 +357,7 @@ source_set("unit_tests") {
     "service/mock_url_loader.h",
     "service/server_url_fetcher_unittest.cc",
     "service/service_impl_unittest.cc",
-    "service/service_request_sender_unittest.cc",
+    "service/service_request_sender_impl_unittest.cc",
     "string_conversions_util_unittest.cc",
     "test_util.cc",
     "test_util.h",

components/autofill_assistant/browser/service/mock_service_request_sender.cc

@@ -6,13 +6,7 @@
 
 namespace autofill_assistant {
 
-MockServiceRequestSender::MockServiceRequestSender()
-    : ServiceRequestSender(/* context = */ nullptr,
-                           /* access_token_fetcher = */ nullptr,
-                           /* loader_factory = */ nullptr,
-                           /* api_key = */ std::string("fake_api_key"),
-                           /* auth_enabled = */ false,
-                           /* disable_auth_if_no_access_token = */ true) {}
+MockServiceRequestSender::MockServiceRequestSender() = default;
 MockServiceRequestSender::~MockServiceRequestSender() = default;
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/service/service_impl.cc

@@ -15,6 +15,7 @@
 #include "components/autofill_assistant/browser/protocol_utils.h"
 #include "components/autofill_assistant/browser/service/api_key_fetcher.h"
 #include "components/autofill_assistant/browser/service/server_url_fetcher.h"
+#include "components/autofill_assistant/browser/service/service_request_sender_impl.h"
 #include "components/autofill_assistant/browser/switches.h"
 #include "components/autofill_assistant/browser/trigger_context.h"
 #include "content/public/browser/browser_context.h"
@@ -31,7 +32,7 @@ std::unique_ptr<ServiceImpl> ServiceImpl::Create(
     content::BrowserContext* context,
     Client* client) {
   ServerUrlFetcher url_fetcher{ServerUrlFetcher::GetDefaultServerUrl()};
-  auto request_sender = std::make_unique<ServiceRequestSender>(
+  auto request_sender = std::make_unique<ServiceRequestSenderImpl>(
       context, client->GetAccessTokenFetcher(),
       std::make_unique<NativeURLLoaderFactory>(),
       ApiKeyFetcher().GetAPIKey(client->GetChannel()),

components/autofill_assistant/browser/service/service_request_sender.cc

@@ -4,212 +4,9 @@
 
 #include "components/autofill_assistant/browser/service/service_request_sender.h"
 
-#include "base/strings/strcat.h"
-#include "content/public/browser/browser_context.h"
-#include "content/public/browser/storage_partition.h"
-#include "net/base/load_flags.h"
-#include "net/http/http_request_headers.h"
-#include "net/http/http_status_code.h"
-#include "net/traffic_annotation/network_traffic_annotation.h"
-#include "services/network/public/cpp/resource_request.h"
-#include "services/network/public/cpp/simple_url_loader.h"
-
-namespace {
-
-constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
-    net::DefineNetworkTrafficAnnotation("autofill_service", R"(
-        semantics {
-          sender: "Autofill Assistant"
-          description:
-            "Chromium posts requests to autofill assistant server to get
-            scripts for a URL."
-          trigger:
-            "Matching URL."
-          data: "None."
-          destination: GOOGLE_OWNED_SERVICE
-        }
-        policy {
-          cookies_allowed: NO
-          setting:
-            "This feature can be disabled in settings."
-          policy_exception_justification: "Not implemented."
-        })");
-
-void OnURLLoaderComplete(
-    autofill_assistant::ServiceRequestSender::ResponseCallback callback,
-    std::unique_ptr<::network::SimpleURLLoader> loader,
-    std::unique_ptr<std::string> response_body) {
-  std::string response_str;
-  if (response_body != nullptr) {
-    response_str = std::move(*response_body);
-  }
-
-  int response_code = 0;
-  if (loader->ResponseInfo() && loader->ResponseInfo()->headers) {
-    response_code = loader->ResponseInfo()->headers->response_code();
-  }
-  std::move(callback).Run(response_code, response_str);
-}
-
-std::unique_ptr<::network::ResourceRequest> CreateResourceRequest(GURL url) {
-  auto resource_request = std::make_unique<::network::ResourceRequest>();
-  resource_request->url = url;
-  resource_request->method = "POST";
-  resource_request->redirect_mode = ::network::mojom::RedirectMode::kError;
-  resource_request->credentials_mode = ::network::mojom::CredentialsMode::kOmit;
-  return resource_request;
-}
-
-void SendRequestImpl(
-    std::unique_ptr<::network::ResourceRequest> request,
-    const std::string& request_body,
-    content::BrowserContext* context,
-    autofill_assistant::SimpleURLLoaderFactory* loader_factory,
-    autofill_assistant::ServiceRequestSender::ResponseCallback callback) {
-  auto loader =
-      loader_factory->CreateLoader(std::move(request), kTrafficAnnotation);
-  loader->AttachStringForUpload(request_body, "application/x-protobuffer");
-#ifdef DEBUG
-  loader->SetAllowHttpErrorResults(true);
-#endif
-  loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
-      content::BrowserContext::GetDefaultStoragePartition(context)
-          ->GetURLLoaderFactoryForBrowserProcess()
-          .get(),
-      base::BindOnce(&OnURLLoaderComplete, std::move(callback),
-                     std::move(loader)));
-}
-
-void SendRequestNoAuth(
-    const GURL& url,
-    const std::string& request_body,
-    content::BrowserContext* context,
-    autofill_assistant::SimpleURLLoaderFactory* loader_factory,
-    const std::string& api_key,
-    autofill_assistant::ServiceRequestSender::ResponseCallback callback) {
-  if (callback.IsCancelled()) {
-    return;
-  }
-  if (api_key.empty()) {
-    LOG(ERROR) << "no api key provided";
-    std::move(callback).Run(net::HTTP_UNAUTHORIZED, std::string());
-    return;
-  }
-
-  std::string query_str = base::StrCat({"key=", api_key});
-  // query_str must remain valid until ReplaceComponents() has returned.
-  url::StringPieceReplacements<std::string> add_key;
-  add_key.SetQueryStr(query_str);
-  GURL modified_url = url.ReplaceComponents(add_key);
-
-  VLOG(2) << "Sending request with api key to backend";
-  SendRequestImpl(CreateResourceRequest(modified_url), request_body, context,
-                  loader_factory, std::move(callback));
-}
-
-}  // namespace
-
 namespace autofill_assistant {
 
-ServiceRequestSender::ServiceRequestSender(
-    content::BrowserContext* context,
-    AccessTokenFetcher* access_token_fetcher,
-    std::unique_ptr<SimpleURLLoaderFactory> loader_factory,
-    const std::string& api_key,
-    bool auth_enabled,
-    bool disable_auth_if_no_access_token)
-    : context_(context),
-      access_token_fetcher_(access_token_fetcher),
-      loader_factory_(std::move(loader_factory)),
-      api_key_(api_key),
-      auth_enabled_(auth_enabled),
-      disable_auth_if_no_access_token_(disable_auth_if_no_access_token) {
-  DCHECK(!auth_enabled || access_token_fetcher != nullptr);
-  DCHECK(auth_enabled || !api_key.empty());
-}
+ServiceRequestSender::ServiceRequestSender() = default;
 ServiceRequestSender::~ServiceRequestSender() = default;
 
-void ServiceRequestSender::SendRequest(const GURL& url,
-                                       const std::string& request_body,
-                                       ResponseCallback callback) {
-  if (auth_enabled_ && access_token_fetcher_ == nullptr) {
-    LOG(ERROR) << "auth requested, but no access token fetcher provided";
-    std::move(callback).Run(net::HTTP_UNAUTHORIZED, std::string());
-    return;
-  }
-  if (auth_enabled_) {
-    access_token_fetcher_->FetchAccessToken(
-        base::BindOnce(&ServiceRequestSender::OnFetchAccessToken,
-                       weak_ptr_factory_.GetWeakPtr(), url, request_body,
-                       std::move(callback)));
-    return;
-  }
-
-  SendRequestNoAuth(url, request_body, context_, loader_factory_.get(),
-                    api_key_, std::move(callback));
-}
-
-void ServiceRequestSender::OnFetchAccessToken(GURL url,
-                                              std::string request_body,
-                                              ResponseCallback callback,
-                                              bool access_token_fetched,
-                                              const std::string& access_token) {
-  if (!access_token_fetched || access_token.empty()) {
-    if (disable_auth_if_no_access_token_) {
-      // Give up on authentication for this run. Without access token, requests
-      // might be successful or rejected, depending on the server configuration.
-      auth_enabled_ = false;
-      VLOG(1) << "No access token, falling back to api key";
-      SendRequestNoAuth(url, request_body, context_, loader_factory_.get(),
-                        api_key_, std::move(callback));
-      return;
-    }
-    VLOG(1) << "No access token, but disable_auth_if_no_access_token not set";
-    std::move(callback).Run(net::HTTP_UNAUTHORIZED, std::string());
-    return;
-  }
-
-  SendRequestAuth(url, request_body, access_token, std::move(callback));
-}
-
-void ServiceRequestSender::SendRequestAuth(const GURL& url,
-                                           const std::string& request_body,
-                                           const std::string& access_token,
-                                           ResponseCallback callback) {
-  if (callback.IsCancelled()) {
-    return;
-  }
-  auto resource_request = CreateResourceRequest(url);
-  resource_request->headers.SetHeader("Authorization",
-                                      base::StrCat({"Bearer ", access_token}));
-
-  if (!retried_with_fresh_access_token_) {
-    callback = base::BindOnce(&ServiceRequestSender::RetryIfUnauthorized,
-                              weak_ptr_factory_.GetWeakPtr(), url, access_token,
-                              request_body, std::move(callback));
-  }
-  VLOG(2) << "Sending request with access token to backend";
-  SendRequestImpl(std::move(resource_request), request_body, context_,
-                  loader_factory_.get(), std::move(callback));
-}
-
-void ServiceRequestSender::RetryIfUnauthorized(const GURL& url,
-                                               const std::string& access_token,
-                                               const std::string& request_body,
-                                               ResponseCallback callback,
-                                               int http_status,
-                                               const std::string& response) {
-  // On first UNAUTHORIZED error, invalidate access token and try again.
-  if (auth_enabled_ && http_status == net::HTTP_UNAUTHORIZED) {
-    VLOG(1) << "Request with access token returned with 401 UNAUTHORIZED, "
-               "fetching a fresh access token and trying again";
-    DCHECK(!retried_with_fresh_access_token_);
-    retried_with_fresh_access_token_ = true;
-    access_token_fetcher_->InvalidateAccessToken(access_token);
-    SendRequest(url, request_body, std::move(callback));
-    return;
-  }
-  std::move(callback).Run(http_status, response);
-}
-
 }  // namespace autofill_assistant

components/autofill_assistant/browser/service/service_request_sender.h

@@ -5,20 +5,11 @@
 #ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_SERVICE_REQUEST_SENDER_H_
 #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_SERVICE_REQUEST_SENDER_H_
 
-#include <memory>
 #include <string>
 
 #include "base/callback.h"
-#include "base/memory/weak_ptr.h"
-#include "components/autofill_assistant/browser/service/access_token_fetcher.h"
-#include "components/autofill_assistant/browser/service/simple_url_loader_factory.h"
-#include "content/public/browser/browser_context.h"
 #include "url/gurl.h"
 
-namespace content {
-class BrowserContext;
-}  // namespace content
-
 namespace autofill_assistant {
 
 class ServiceRequestSender {
@@ -26,66 +17,14 @@ class ServiceRequestSender {
   using ResponseCallback =
       base::OnceCallback<void(int http_status, const std::string& response)>;
 
-  // Constructor. |access_token_fetcher| is optional if |auth_enabled| is false.
-  // Pointers to |context| and, if provided, |access_token_fetcher| must remain
-  // valid during the lifetime of this instance.
-
-  // If |disable_auth_if_no_access_token| is true, authentication will
-  // automatically be disabled in case fetching the access token fails.
-  ServiceRequestSender(content::BrowserContext* context,
-                       AccessTokenFetcher* access_token_fetcher,
-                       std::unique_ptr<SimpleURLLoaderFactory> loader_factory,
-                       const std::string& api_key,
-                       bool auth_enabled,
-                       bool disable_auth_if_no_access_token);
+  ServiceRequestSender();
   virtual ~ServiceRequestSender();
-  ServiceRequestSender(const ServiceRequestSender&) = delete;
-  ServiceRequestSender& operator=(const ServiceRequestSender&) = delete;
 
-  // Sends |request_body| to |url|. Depending on configuration, the request
-  // will be authenticated either with an Oauth access token or the api key.
-  // Returns the http status code and the response itself. If the returned http
-  // headers could not be parsed, the http code will be 0.
-  //
-  // When an auth-request first fails with a 401, the access token is
-  // invalidated and fetched again. If the request fails again, the request
-  // is considered failed and the callback is invoked.
+  // Sends |request_body| to |url|. Returns the http status code and the
+  // response itself.
   virtual void SendRequest(const GURL& url,
                            const std::string& request_body,
-                           ResponseCallback callback);
-
- private:
-  void SendRequestAuth(const GURL& url,
-                       const std::string& request_body,
-                       const std::string& access_token,
-                       ResponseCallback callback);
-
-  void RetryIfUnauthorized(const GURL& url,
-                           const std::string& access_token,
-                           const std::string& request_body,
-                           ResponseCallback callback,
-                           int http_status,
-                           const std::string& response);
-
-  void OnFetchAccessToken(GURL url,
-                          std::string request_body,
-                          ResponseCallback callback,
-                          bool access_token_fetched,
-                          const std::string& access_token);
-
-  content::BrowserContext* context_ = nullptr;
-  AccessTokenFetcher* access_token_fetcher_ = nullptr;
-  std::unique_ptr<SimpleURLLoaderFactory> loader_factory_;
-
-  // API key to add to the URL of unauthenticated requests.
-  std::string api_key_;
-
-  // Whether requests should be authenticated.
-  bool auth_enabled_ = true;
-
-  bool disable_auth_if_no_access_token_ = true;
-  bool retried_with_fresh_access_token_ = false;
-  base::WeakPtrFactory<ServiceRequestSender> weak_ptr_factory_{this};
+                           ResponseCallback callback) = 0;
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/service/service_request_sender_impl.cc

+// Copyright 2020 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 "components/autofill_assistant/browser/service/service_request_sender_impl.h"
+
+#include "base/strings/strcat.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/storage_partition.h"
+#include "net/base/load_flags.h"
+#include "net/http/http_request_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+
+namespace {
+
+constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
+    net::DefineNetworkTrafficAnnotation("autofill_service", R"(
+        semantics {
+          sender: "Autofill Assistant"
+          description:
+            "Chromium posts requests to autofill assistant server to get
+            scripts for a URL."
+          trigger:
+            "Matching URL."
+          data: "None."
+          destination: GOOGLE_OWNED_SERVICE
+        }
+        policy {
+          cookies_allowed: NO
+          setting:
+            "This feature can be disabled in settings."
+          policy_exception_justification: "Not implemented."
+        })");
+
+void OnURLLoaderComplete(
+    autofill_assistant::ServiceRequestSender::ResponseCallback callback,
+    std::unique_ptr<::network::SimpleURLLoader> loader,
+    std::unique_ptr<std::string> response_body) {
+  std::string response_str;
+  if (response_body != nullptr) {
+    response_str = std::move(*response_body);
+  }
+
+  int response_code = 0;
+  if (loader->ResponseInfo() && loader->ResponseInfo()->headers) {
+    response_code = loader->ResponseInfo()->headers->response_code();
+  }
+  std::move(callback).Run(response_code, response_str);
+}
+
+std::unique_ptr<::network::ResourceRequest> CreateResourceRequest(GURL url) {
+  auto resource_request = std::make_unique<::network::ResourceRequest>();
+  resource_request->url = url;
+  resource_request->method = "POST";
+  resource_request->redirect_mode = ::network::mojom::RedirectMode::kError;
+  resource_request->credentials_mode = ::network::mojom::CredentialsMode::kOmit;
+  return resource_request;
+}
+
+void SendRequestImpl(
+    std::unique_ptr<::network::ResourceRequest> request,
+    const std::string& request_body,
+    content::BrowserContext* context,
+    autofill_assistant::SimpleURLLoaderFactory* loader_factory,
+    autofill_assistant::ServiceRequestSender::ResponseCallback callback) {
+  auto loader =
+      loader_factory->CreateLoader(std::move(request), kTrafficAnnotation);
+  loader->AttachStringForUpload(request_body, "application/x-protobuffer");
+#ifdef DEBUG
+  loader->SetAllowHttpErrorResults(true);
+#endif
+  loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      content::BrowserContext::GetDefaultStoragePartition(context)
+          ->GetURLLoaderFactoryForBrowserProcess()
+          .get(),
+      base::BindOnce(&OnURLLoaderComplete, std::move(callback),
+                     std::move(loader)));
+}
+
+void SendRequestNoAuth(
+    const GURL& url,
+    const std::string& request_body,
+    content::BrowserContext* context,
+    autofill_assistant::SimpleURLLoaderFactory* loader_factory,
+    const std::string& api_key,
+    autofill_assistant::ServiceRequestSender::ResponseCallback callback) {
+  if (callback.IsCancelled()) {
+    return;
+  }
+  if (api_key.empty()) {
+    LOG(ERROR) << "no api key provided";
+    std::move(callback).Run(net::HTTP_UNAUTHORIZED, std::string());
+    return;
+  }
+
+  std::string query_str = base::StrCat({"key=", api_key});
+  // query_str must remain valid until ReplaceComponents() has returned.
+  url::StringPieceReplacements<std::string> add_key;
+  add_key.SetQueryStr(query_str);
+  GURL modified_url = url.ReplaceComponents(add_key);
+
+  VLOG(2) << "Sending request with api key to backend";
+  SendRequestImpl(CreateResourceRequest(modified_url), request_body, context,
+                  loader_factory, std::move(callback));
+}
+
+}  // namespace
+
+namespace autofill_assistant {
+
+ServiceRequestSenderImpl::ServiceRequestSenderImpl(
+    content::BrowserContext* context,
+    AccessTokenFetcher* access_token_fetcher,
+    std::unique_ptr<SimpleURLLoaderFactory> loader_factory,
+    const std::string& api_key,
+    bool auth_enabled,
+    bool disable_auth_if_no_access_token)
+    : context_(context),
+      access_token_fetcher_(access_token_fetcher),
+      loader_factory_(std::move(loader_factory)),
+      api_key_(api_key),
+      auth_enabled_(auth_enabled),
+      disable_auth_if_no_access_token_(disable_auth_if_no_access_token) {
+  DCHECK(!auth_enabled || access_token_fetcher != nullptr);
+  DCHECK(auth_enabled || !api_key.empty());
+}
+ServiceRequestSenderImpl::~ServiceRequestSenderImpl() = default;
+
+void ServiceRequestSenderImpl::SendRequest(const GURL& url,
+                                           const std::string& request_body,
+                                           ResponseCallback callback) {
+  if (auth_enabled_ && access_token_fetcher_ == nullptr) {
+    LOG(ERROR) << "auth requested, but no access token fetcher provided";
+    std::move(callback).Run(net::HTTP_UNAUTHORIZED, std::string());
+    return;
+  }
+  if (auth_enabled_) {
+    access_token_fetcher_->FetchAccessToken(
+        base::BindOnce(&ServiceRequestSenderImpl::OnFetchAccessToken,
+                       weak_ptr_factory_.GetWeakPtr(), url, request_body,
+                       std::move(callback)));
+    return;
+  }
+
+  SendRequestNoAuth(url, request_body, context_, loader_factory_.get(),
+                    api_key_, std::move(callback));
+}
+
+void ServiceRequestSenderImpl::OnFetchAccessToken(
+    GURL url,
+    std::string request_body,
+    ResponseCallback callback,
+    bool access_token_fetched,
+    const std::string& access_token) {
+  if (!access_token_fetched || access_token.empty()) {
+    if (disable_auth_if_no_access_token_) {
+      // Give up on authentication for this run. Without access token, requests
+      // might be successful or rejected, depending on the server configuration.
+      auth_enabled_ = false;
+      VLOG(1) << "No access token, falling back to api key";
+      SendRequestNoAuth(url, request_body, context_, loader_factory_.get(),
+                        api_key_, std::move(callback));
+      return;
+    }
+    VLOG(1) << "No access token, but disable_auth_if_no_access_token not set";
+    std::move(callback).Run(net::HTTP_UNAUTHORIZED, std::string());
+    return;
+  }
+
+  SendRequestAuth(url, request_body, access_token, std::move(callback));
+}
+
+void ServiceRequestSenderImpl::SendRequestAuth(const GURL& url,
+                                               const std::string& request_body,
+                                               const std::string& access_token,
+                                               ResponseCallback callback) {
+  if (callback.IsCancelled()) {
+    return;
+  }
+  auto resource_request = CreateResourceRequest(url);
+  resource_request->headers.SetHeader("Authorization",
+                                      base::StrCat({"Bearer ", access_token}));
+
+  if (!retried_with_fresh_access_token_) {
+    callback = base::BindOnce(&ServiceRequestSenderImpl::RetryIfUnauthorized,
+                              weak_ptr_factory_.GetWeakPtr(), url, access_token,
+                              request_body, std::move(callback));
+  }
+  VLOG(2) << "Sending request with access token to backend";
+  SendRequestImpl(std::move(resource_request), request_body, context_,
+                  loader_factory_.get(), std::move(callback));
+}
+
+void ServiceRequestSenderImpl::RetryIfUnauthorized(
+    const GURL& url,
+    const std::string& access_token,
+    const std::string& request_body,
+    ResponseCallback callback,
+    int http_status,
+    const std::string& response) {
+  // On first UNAUTHORIZED error, invalidate access token and try again.
+  if (auth_enabled_ && http_status == net::HTTP_UNAUTHORIZED) {
+    VLOG(1) << "Request with access token returned with 401 UNAUTHORIZED, "
+               "fetching a fresh access token and trying again";
+    DCHECK(!retried_with_fresh_access_token_);
+    retried_with_fresh_access_token_ = true;
+    access_token_fetcher_->InvalidateAccessToken(access_token);
+    SendRequest(url, request_body, std::move(callback));
+    return;
+  }
+  std::move(callback).Run(http_status, response);
+}
+
+}  // namespace autofill_assistant

components/autofill_assistant/browser/service/service_request_sender_impl.h

+// Copyright 2020 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.
+
+#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_SERVICE_REQUEST_SENDER_IMPL_H_
+#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_SERVICE_REQUEST_SENDER_IMPL_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "components/autofill_assistant/browser/service/access_token_fetcher.h"
+#include "components/autofill_assistant/browser/service/service_request_sender.h"
+#include "components/autofill_assistant/browser/service/simple_url_loader_factory.h"
+#include "content/public/browser/browser_context.h"
+#include "url/gurl.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace autofill_assistant {
+
+class ServiceRequestSenderImpl : public ServiceRequestSender {
+ public:
+  // Constructor. |access_token_fetcher| is optional if |auth_enabled| is false.
+  // Pointers to |context| and, if provided, |access_token_fetcher| must remain
+  // valid during the lifetime of this instance.
+  // If |disable_auth_if_no_access_token| is true, authentication will
+  // automatically be disabled in case fetching the access token fails.
+  ServiceRequestSenderImpl(
+      content::BrowserContext* context,
+      AccessTokenFetcher* access_token_fetcher,
+      std::unique_ptr<SimpleURLLoaderFactory> loader_factory,
+      const std::string& api_key,
+      bool auth_enabled,
+      bool disable_auth_if_no_access_token);
+  ~ServiceRequestSenderImpl() override;
+  ServiceRequestSenderImpl(const ServiceRequestSenderImpl&) = delete;
+  ServiceRequestSenderImpl& operator=(const ServiceRequestSenderImpl&) = delete;
+
+  // Sends |request_body| to |url|. Depending on configuration, the request
+  // will be authenticated either with an Oauth access token or the api key.
+  // Returns the http status code and the response itself. If the returned http
+  // headers could not be parsed, the http code will be 0.
+  //
+  // When an auth-request first fails with a 401, the access token is
+  // invalidated and fetched again. If the request fails again, the request
+  // is considered failed and the callback is invoked.
+  void SendRequest(const GURL& url,
+                   const std::string& request_body,
+                   ResponseCallback callback) override;
+
+ private:
+  void SendRequestAuth(const GURL& url,
+                       const std::string& request_body,
+                       const std::string& access_token,
+                       ResponseCallback callback);
+
+  void RetryIfUnauthorized(const GURL& url,
+                           const std::string& access_token,
+                           const std::string& request_body,
+                           ResponseCallback callback,
+                           int http_status,
+                           const std::string& response);
+
+  void OnFetchAccessToken(GURL url,
+                          std::string request_body,
+                          ResponseCallback callback,
+                          bool access_token_fetched,
+                          const std::string& access_token);
+
+  content::BrowserContext* context_ = nullptr;
+  AccessTokenFetcher* access_token_fetcher_ = nullptr;
+  std::unique_ptr<SimpleURLLoaderFactory> loader_factory_;
+
+  // API key to add to the URL of unauthenticated requests.
+  std::string api_key_;
+
+  // Whether requests should be authenticated.
+  bool auth_enabled_ = true;
+
+  bool disable_auth_if_no_access_token_ = true;
+  bool retried_with_fresh_access_token_ = false;
+  base::WeakPtrFactory<ServiceRequestSenderImpl> weak_ptr_factory_{this};
+};
+
+}  // namespace autofill_assistant
+
+#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_SERVICE_REQUEST_SENDER_IMPL_H_

components/autofill_assistant/browser/service/service_request_sender_unittest.cc → components/autofill_assistant/browser/service/service_request_sender_impl_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/autofill_assistant/browser/service/service_request_sender.h"
+#include "components/autofill_assistant/browser/service/service_request_sender_impl.h"
 
 #include <memory>
 #include <vector>
@@ -46,10 +46,10 @@ std::unique_ptr<::network::mojom::URLResponseHead> CreateResponseInfo(
   return response;
 }
 
-class ServiceRequestSenderTest : public testing::Test {
+class ServiceRequestSenderImplTest : public testing::Test {
  public:
-  ServiceRequestSenderTest() = default;
-  ~ServiceRequestSenderTest() override = default;
+  ServiceRequestSenderImplTest() = default;
+  ~ServiceRequestSenderImplTest() override = default;
 
  protected:
   base::MockCallback<base::OnceCallback<void(int, const std::string&)>>
@@ -61,7 +61,7 @@ class ServiceRequestSenderTest : public testing::Test {
   NiceMock<MockAccessTokenFetcher> mock_access_token_fetcher_;
 };
 
-TEST_F(ServiceRequestSenderTest, SendUnauthenticatedRequest) {
+TEST_F(ServiceRequestSenderImplTest, SendUnauthenticatedRequest) {
   auto loader_factory =
       std::make_unique<NiceMock<MockSimpleURLLoaderFactory>>();
   auto loader = std::make_unique<NiceMock<MockURLLoader>>();
@@ -86,7 +86,7 @@ TEST_F(ServiceRequestSenderTest, SendUnauthenticatedRequest) {
       .WillRepeatedly(Return(response_info.get()));
 
   EXPECT_CALL(mock_response_callback_, Run(net::HTTP_OK, "response"));
-  ServiceRequestSender request_sender{
+  ServiceRequestSenderImpl request_sender{
       &context_,
       /* access_token_fetcher = */ nullptr,
       std::move(loader_factory),
@@ -98,7 +98,7 @@ TEST_F(ServiceRequestSenderTest, SendUnauthenticatedRequest) {
                              mock_response_callback_.Get());
 }
 
-TEST_F(ServiceRequestSenderTest, SendAuthenticatedRequest) {
+TEST_F(ServiceRequestSenderImplTest, SendAuthenticatedRequest) {
   auto loader_factory =
       std::make_unique<NiceMock<MockSimpleURLLoaderFactory>>();
   auto loader = std::make_unique<NiceMock<MockURLLoader>>();
@@ -129,7 +129,7 @@ TEST_F(ServiceRequestSenderTest, SendAuthenticatedRequest) {
   EXPECT_CALL(mock_access_token_fetcher_, InvalidateAccessToken).Times(0);
 
   EXPECT_CALL(mock_response_callback_, Run(net::HTTP_OK, "response"));
-  ServiceRequestSender request_sender{
+  ServiceRequestSenderImpl request_sender{
       &context_,
       /* access_token_fetcher = */ &mock_access_token_fetcher_,
       std::move(loader_factory),
@@ -141,7 +141,7 @@ TEST_F(ServiceRequestSenderTest, SendAuthenticatedRequest) {
                              mock_response_callback_.Get());
 }
 
-TEST_F(ServiceRequestSenderTest,
+TEST_F(ServiceRequestSenderImplTest,
        AuthRequestFallsBackToApiKeyOnEmptyAccessToken) {
   EXPECT_CALL(mock_access_token_fetcher_, OnFetchAccessToken)
       .Times(1)
@@ -171,7 +171,7 @@ TEST_F(ServiceRequestSenderTest,
       .WillRepeatedly(Return(response_info.get()));
 
   EXPECT_CALL(mock_response_callback_, Run(net::HTTP_OK, "response"));
-  ServiceRequestSender request_sender{
+  ServiceRequestSenderImpl request_sender{
       &context_,
       /* access_token_fetcher = */ &mock_access_token_fetcher_,
       std::move(loader_factory),
@@ -183,7 +183,7 @@ TEST_F(ServiceRequestSenderTest,
                              mock_response_callback_.Get());
 }
 
-TEST_F(ServiceRequestSenderTest,
+TEST_F(ServiceRequestSenderImplTest,
        AuthRequestFallsBackToApiKeyIfFetchingAccessTokenFails) {
   EXPECT_CALL(mock_access_token_fetcher_, OnFetchAccessToken)
       .Times(1)
@@ -214,7 +214,7 @@ TEST_F(ServiceRequestSenderTest,
       .WillRepeatedly(Return(response_info.get()));
 
   EXPECT_CALL(mock_response_callback_, Run(net::HTTP_OK, "response"));
-  ServiceRequestSender request_sender{
+  ServiceRequestSenderImpl request_sender{
       &context_,
       /* access_token_fetcher = */ &mock_access_token_fetcher_,
       std::move(loader_factory),