[Autofill Assistant] Share backend url with extension api code.

Before this patch, the backend url logic was in the android client and
not directly available to the extension code. Since switching backends
is useful during development and testing scenarios, this patch moves the
command line switches to a shared file. The default URL can be in the
service impl and does not need to be part of the client interface. The
patch also cleans up the use of the api keys, which don't need to be
handled in chrome, only the installation channel needs to be part of the
Client interface.

Bug: b/143736397
Change-Id: I30caeffc226e6936d43e0000d897f9bfbf53d9c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129549
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#756192}

chrome/browser/android/autofill_assistant/client_android.cc

@@ -30,6 +30,7 @@
 #include "components/autofill_assistant/browser/access_token_fetcher.h"
 #include "components/autofill_assistant/browser/controller.h"
 #include "components/autofill_assistant/browser/features.h"
+#include "components/autofill_assistant/browser/switches.h"
 #include "components/autofill_assistant/browser/website_login_fetcher_impl.h"
 #include "components/password_manager/content/browser/content_password_manager_driver.h"
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
@@ -40,7 +41,6 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
-#include "google_apis/google_api_keys.h"
 #include "url/gurl.h"
 
 using base::android::AttachCurrentThread;
@@ -48,16 +48,8 @@ using base::android::JavaParamRef;
 using base::android::JavaRef;
 
 namespace autofill_assistant {
-namespace switches {
-const char* const kAutofillAssistantServerKey = "autofill-assistant-key";
-const char* const kAutofillAssistantUrl = "autofill-assistant-url";
-}  // namespace switches
-
 namespace {
 
-const char* const kDefaultAutofillAssistantServerUrl =
-    "https://automate-pa.googleapis.com";
-
 // A direct action that corresponds to pressing the close or cancel button on
 // the UI.
 const char* const kCancelActionName = "cancel";
@@ -106,11 +98,6 @@ ClientAndroid::ClientAndroid(content::WebContents* web_contents)
       java_object_(Java_AutofillAssistantClient_create(
           AttachCurrentThread(),
           reinterpret_cast<intptr_t>(this))) {
-  server_url_ = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-      switches::kAutofillAssistantUrl);
-  if (server_url_.empty()) {
-    server_url_ = kDefaultAutofillAssistantServerUrl;
-  }
 }
 
 ClientAndroid::~ClientAndroid() {
@@ -449,19 +436,8 @@ void ClientAndroid::DestroyUI() {
   ui_controller_android_.reset();
 }
 
-std::string ClientAndroid::GetApiKey() const {
-  std::string api_key;
-  if (google_apis::IsGoogleChromeAPIKeyUsed()) {
-    api_key = chrome::GetChannel() == version_info::Channel::STABLE
-                  ? google_apis::GetAPIKey()
-                  : google_apis::GetNonStableAPIKey();
-  }
-  const auto* command_line = base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kAutofillAssistantServerKey)) {
-    api_key = command_line->GetSwitchValueASCII(
-        switches::kAutofillAssistantServerKey);
-  }
-  return api_key;
+version_info::Channel ClientAndroid::GetChannel() const {
+  return chrome::GetChannel();
 }
 
 std::string ClientAndroid::GetAccountEmailAddress() const {
@@ -504,10 +480,6 @@ WebsiteLoginFetcher* ClientAndroid::GetWebsiteLoginFetcher() const {
   return website_login_fetcher_.get();
 }
 
-std::string ClientAndroid::GetServerUrl() const {
-  return server_url_;
-}
-
 std::string ClientAndroid::GetLocale() const {
   return base::android::GetDefaultLocaleString();
 }
@@ -596,4 +568,4 @@ bool ClientAndroid::NeedsUI() {
 
 WEB_CONTENTS_USER_DATA_KEY_IMPL(ClientAndroid)
 
-}  // namespace autofill_assistant.
+}  // namespace autofill_assistant

chrome/browser/android/autofill_assistant/client_android.h

@@ -96,14 +96,13 @@ class ClientAndroid : public Client,
   // Overrides Client
   void AttachUI() override;
   void DestroyUI() override;
-  std::string GetApiKey() const override;
+  version_info::Channel GetChannel() const override;
   std::string GetAccountEmailAddress() const override;
   AccessTokenFetcher* GetAccessTokenFetcher() override;
   autofill::PersonalDataManager* GetPersonalDataManager() const override;
   password_manager::PasswordManagerClient* GetPasswordManagerClient()
       const override;
   WebsiteLoginFetcher* GetWebsiteLoginFetcher() const override;
-  std::string GetServerUrl() const override;
   std::string GetLocale() const override;
   std::string GetCountryCode() const override;
   DeviceContext GetDeviceContext() const override;
@@ -156,7 +155,6 @@ class ClientAndroid : public Client,
 
   base::OnceCallback<void(bool, const std::string&)>
       fetch_access_token_callback_;
-  std::string server_url_;
 
   base::WeakPtrFactory<ClientAndroid> weak_ptr_factory_{this};
 

chrome/browser/extensions/api/autofill_assistant_private/autofill_assistant_private_api.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/channel_info.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "extensions/browser/event_router_factory.h"
 
@@ -314,10 +315,11 @@ void AutofillAssistantPrivateAPI::AttachUI() {}
 
 void AutofillAssistantPrivateAPI::DestroyUI() {}
 
-std::string AutofillAssistantPrivateAPI::GetApiKey() const {
-  // TODO(crbug.com/1015753): Use chromium's keys and also consider the
-  // autofill-assistant-key here to override that particular key.
-  return "invalid";
+version_info::Channel AutofillAssistantPrivateAPI::GetChannel() const {
+  // TODO(crbug.com/1015753): Make a minimal client impl available in a common
+  // chrome/browser/autofill_assistant and share it with the android client
+  // impl.
+  return chrome::GetChannel();
 }
 
 std::string AutofillAssistantPrivateAPI::GetAccountEmailAddress() const {
@@ -346,13 +348,6 @@ AutofillAssistantPrivateAPI::GetWebsiteLoginFetcher() const {
   return nullptr;
 }
 
-std::string AutofillAssistantPrivateAPI::GetServerUrl() const {
-  // TODO(crbug.com/1015753): Consider the autofill-assistant-url for endpoint
-  // overrides and share the kDefaultAutofillAssistantServerUrl to expose it
-  // here.
-  return "https://automate-pa.googleapis.com";
-}
-
 std::string AutofillAssistantPrivateAPI::GetLocale() const {
   return "en-us";
 }

chrome/browser/extensions/api/autofill_assistant_private/autofill_assistant_private_api.h

@@ -196,7 +196,7 @@ class AutofillAssistantPrivateAPI : public BrowserContextKeyedAPI,
   // autofill_assistant::Client:
   void AttachUI() override;
   void DestroyUI() override;
-  std::string GetApiKey() const override;
+  version_info::Channel GetChannel() const override;
   std::string GetAccountEmailAddress() const override;
   autofill_assistant::AccessTokenFetcher* GetAccessTokenFetcher() override;
   autofill::PersonalDataManager* GetPersonalDataManager() const override;
@@ -204,7 +204,6 @@ class AutofillAssistantPrivateAPI : public BrowserContextKeyedAPI,
       const override;
   autofill_assistant::WebsiteLoginFetcher* GetWebsiteLoginFetcher()
       const override;
-  std::string GetServerUrl() const override;
   std::string GetLocale() const override;
   std::string GetCountryCode() const override;
   autofill_assistant::DeviceContext GetDeviceContext() const override;

components/autofill_assistant/browser/BUILD.gn

@@ -139,6 +139,8 @@ jumbo_static_library("browser") {
     "state.h",
     "string_conversions_util.cc",
     "string_conversions_util.h",
+    "switches.cc",
+    "switches.h",
     "top_padding.cc",
     "top_padding.h",
     "trigger_context.cc",
@@ -219,6 +221,7 @@ static_library("unit_test_support") {
     "//base",
     "//base/test:test_support",
     "//components/autofill/core/browser:test_support",
+    "//components/version_info",
     "//testing/gmock",
     "//testing/gtest",
   ]

components/autofill_assistant/browser/DEPS

@@ -2,7 +2,7 @@ include_rules = [
   "+chrome/android/features/autofill_assistant/test_support_jni_headers",
   "+components/autofill",
   "+components/password_manager/core/browser",
-  "+components/version_info/version_info.h",
+  "+components/version_info",
   "+content/public/browser",
   "+content/public/test",
   "+content/shell/browser/shell.h",

components/autofill_assistant/browser/client.h

@@ -17,7 +17,11 @@ class PersonalDataManager;
 
 namespace password_manager {
 class PasswordManagerClient;
-}
+}  // namespace password_manager
+
+namespace version_info {
+enum class Channel;
+}  // namespace version_info
 
 namespace autofill_assistant {
 class AccessTokenFetcher;
@@ -38,8 +42,9 @@ class Client {
   // Destroys the UI immediately.
   virtual void DestroyUI() = 0;
 
-  // Returns the API key to be used for requests to the backend.
-  virtual std::string GetApiKey() const = 0;
+  // Returns the channel for the installation (canary, dev, beta, stable).
+  // Returns unknown otherwise.
+  virtual version_info::Channel GetChannel() const = 0;
 
   // Returns the e-mail address that corresponds to the auth credentials. Might
   // be empty.
@@ -58,9 +63,6 @@ class Client {
   // Returns the currently active login fetcher.
   virtual WebsiteLoginFetcher* GetWebsiteLoginFetcher() const = 0;
 
-  // Returns the server URL to be used for requests to the backend.
-  virtual std::string GetServerUrl() const = 0;
-
   // Returns the locale.
   virtual std::string GetLocale() const = 0;
 

components/autofill_assistant/browser/mock_client.h

@@ -11,6 +11,7 @@
 #include "components/autofill_assistant/browser/metrics.h"
 #include "components/autofill_assistant/browser/mock_personal_data_manager.h"
 #include "components/autofill_assistant/browser/website_login_fetcher.h"
+#include "components/version_info/channel.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace autofill_assistant {
@@ -20,8 +21,7 @@ class MockClient : public Client {
   MockClient();
   ~MockClient();
 
-  MOCK_CONST_METHOD0(GetApiKey, std::string());
-  MOCK_CONST_METHOD0(GetServerUrl, std::string());
+  MOCK_CONST_METHOD0(GetChannel, version_info::Channel());
   MOCK_CONST_METHOD0(GetLocale, std::string());
   MOCK_CONST_METHOD0(GetCountryCode, std::string());
   MOCK_CONST_METHOD0(GetDeviceContext, DeviceContext());

components/autofill_assistant/browser/service_impl.cc

@@ -15,24 +15,24 @@
 #include "base/strings/stringprintf.h"
 #include "components/autofill_assistant/browser/client.h"
 #include "components/autofill_assistant/browser/protocol_utils.h"
+#include "components/autofill_assistant/browser/switches.h"
 #include "components/autofill_assistant/browser/trigger_context.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/storage_partition.h"
 #include "crypto/sha2.h"
+#include "google_apis/google_api_keys.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 "url/url_canon_stdstring.h"
 
+namespace autofill_assistant {
 namespace {
-namespace switches {
-// --autofill_assistant-auth=false disables authentication. This is only useful
-// during development, as prod instances require authentication.
-const char* const kAutofillAssistantAuth = "autofill-assistant-auth";
-}  // namespace switches
 
+const char* const kDefaultAutofillAssistantServerUrl =
+    "https://automate-pa.googleapis.com";
 const char* const kScriptEndpoint = "/v1/supportsSite2";
 const char* const kActionEndpoint = "/v1/actions2";
 
@@ -54,21 +54,45 @@ net::NetworkTrafficAnnotationTag traffic_annotation =
             "This feature can be disabled in settings."
           policy_exception_justification: "Not implemented."
         })");
-}  // namespace
 
-namespace autofill_assistant {
+std::string GetAPIKey(version_info::Channel channel) {
+  const auto* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kAutofillAssistantServerKey)) {
+    return command_line->GetSwitchValueASCII(
+        switches::kAutofillAssistantServerKey);
+  }
+
+  if (google_apis::IsGoogleChromeAPIKeyUsed()) {
+    return channel == version_info::Channel::STABLE
+               ? google_apis::GetAPIKey()
+               : google_apis::GetNonStableAPIKey();
+  }
+  return "";
+}
+
+std::string GetServerUrl() {
+  std::string server_url =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kAutofillAssistantUrl);
+  if (server_url.empty()) {
+    server_url = kDefaultAutofillAssistantServerUrl;
+  }
+  return server_url;
+}
+
+}  // namespace
 
 // static
 std::unique_ptr<ServiceImpl> ServiceImpl::Create(
     content::BrowserContext* context,
     Client* client) {
-  GURL server_url(client->GetServerUrl());
+  GURL server_url(GetServerUrl());
   DCHECK(server_url.is_valid());
 
   return std::make_unique<ServiceImpl>(
-      client->GetApiKey(), server_url, context, client->GetAccessTokenFetcher(),
-      client->GetLocale(), client->GetCountryCode(), client->GetDeviceContext(),
-      client);
+      GetAPIKey(client->GetChannel()), server_url, context,
+      client->GetAccessTokenFetcher(), client->GetLocale(),
+      client->GetCountryCode(), client->GetDeviceContext(), client);
 }
 
 ServiceImpl::ServiceImpl(const std::string& api_key,

components/autofill_assistant/browser/service_impl_unittest.cc

@@ -19,9 +19,6 @@ using ::testing::Return;
 class ServiceImplTest : public ::testing::Test {
  public:
   ServiceImplTest() {
-    ON_CALL(mock_client_, GetServerUrl)
-        .WillByDefault(Return("https://www.google.com/"));
-
     service_impl_ = ServiceImpl::Create(nullptr, &mock_client_);
   }
 

components/autofill_assistant/browser/switches.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/switches.h"
+
+namespace autofill_assistant {
+namespace switches {
+
+// Sets the API key to be used instead of Chrome's default key when sending
+// requests to the backend.
+const char kAutofillAssistantServerKey[] = "autofill-assistant-key";
+
+// Overrides the default backend URL.
+const char kAutofillAssistantUrl[] = "autofill-assistant-url";
+
+// Disables authentication when set to false. This is only useful
+// during development, as prod instances require authentication.
+const char kAutofillAssistantAuth[] = "autofill-assistant-auth";
+
+}  // namespace switches
+}  // namespace autofill_assistant

components/autofill_assistant/browser/switches.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_SWITCHES_H_
+#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SWITCHES_H_
+
+namespace autofill_assistant {
+namespace switches {
+
+extern const char kAutofillAssistantServerKey[];
+extern const char kAutofillAssistantUrl[];
+extern const char kAutofillAssistantAuth[];
+
+}  // namespace switches
+}  // namespace autofill_assistant
+
+#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SWITCHES_H_