Reland "Add Assistant integration tests using a fake S3 server."

This is a reland of crrev.com/c/1894296

The commit was reverted in crrev.com/c/1971041 because the new browser
tests failed when run by the |linux-chromeos-chrome| trybot.

The tests failed because the |fake_s3_server_main| binary was not copied
over to the sandboxes used by the |linux-chromeos-chrome| trybot while
it runs its tests.

This is fixed by adding the |fake_s3_server_main| as a runtime dependency
of the |browser_unittests| build target.

Original change's description:
> Add Assistant integration tests using a fake S3 server.
>
> These tests run as browser tests, use the real libassistant code and use a fake
> s3 server to replay previously recorded conversations.
>
> This allows the Assistant to run CQ tests that verify the basic interactions
> are not broken.
>
> More specifically, this uses a FakeS3Server library that was previously
> created by the LibAssistant team. On the ChromeOS side I added hooks to
> tell LibAssistant to use the FakeS3Server, and a test-fixture to start/stop
> the fake S3 server during the browsertests.
>
> BUG: b/141264108
> TESTS: in target |browser_tests| with names |AssistantBrowserTest.*|
>
> Change-Id: I89985c9e34eaa21a17eef69fa5b472fa39486d30
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894296
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
> Cr-Commit-Position: refs/heads/master@{#725214}

TBR=xiyuan@chromium.org,sammc@chromium.org, xiaohuic@chromium.org
TBRing reviewers:
   - Note There were no changes in any file since crrev.com/c/1894296 except |chrome/test/BUILD.gn|
   - xiyuan@ please look at assistant_test_api.h
   - sammc@ please look at assistant.mojom
   - xiaohuic@ please look at the assistant code

Change-Id: I514daac006c1254db26adf8b0fd3f6be652f4581
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1971225Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/master@{#725999}

ash/app_list/views/assistant/assistant_test_api_impl.cc

@@ -10,9 +10,11 @@
 #include "ash/app_list/views/app_list_view.h"
 #include "ash/app_list/views/contents_view.h"
 #include "ash/assistant/ui/assistant_view_ids.h"
+#include "ash/public/cpp/assistant/assistant_state.h"
 #include "ash/public/cpp/tablet_mode.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
+#include "base/bind.h"
 #include "components/prefs/pref_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
@@ -40,7 +42,7 @@ void AssistantTestApiImpl::DisableAnimations() {
 }
 
 bool AssistantTestApiImpl::IsVisible() {
-  return page_view()->GetVisible();
+  return AppListViewsHaveBeenCreated() && page_view()->GetVisible();
 }
 
 void AssistantTestApiImpl::SendTextQuery(const std::string& query) {
@@ -64,6 +66,10 @@ views::View* AssistantTestApiImpl::main_view() {
   return page_view()->GetViewByID(AssistantViewID::kMainView);
 }
 
+views::View* AssistantTestApiImpl::ui_element_container() {
+  return page_view()->GetViewByID(AssistantViewID::kUiElementContainer);
+}
+
 views::Textfield* AssistantTestApiImpl::input_text_field() {
   return static_cast<views::Textfield*>(
       page_view()->GetViewByID(AssistantViewID::kTextQueryField));
@@ -93,9 +99,19 @@ views::View* AssistantTestApiImpl::app_list_view() {
   return static_cast<views::View*>(contents_view()->app_list_view());
 }
 
-void AssistantTestApiImpl::EnableAssistant() {
+aura::Window* AssistantTestApiImpl::root_window() {
+  return Shell::Get()->GetPrimaryRootWindow();
+}
+
+void AssistantTestApiImpl::SetAssistantEnabled(bool value) {
   Shell::Get()->session_controller()->GetPrimaryUserPrefService()->SetBoolean(
-      chromeos::assistant::prefs::kAssistantEnabled, true);
+      chromeos::assistant::prefs::kAssistantEnabled, value);
+
+  // Ensure the value has taken effect.
+  ASSERT_EQ(GetAssistantState()->settings_enabled(), value)
+      << "Changing this preference did not take effect immediately, which will "
+         "cause timing issues in this test. If this trace is seen we must add "
+         "a waiter here to wait for the new state to take effect.";
 }
 
 void AssistantTestApiImpl::SetTabletMode(bool enable) {
@@ -105,6 +121,16 @@ void AssistantTestApiImpl::SetTabletMode(bool enable) {
 void AssistantTestApiImpl::SetPreferVoice(bool value) {
   Shell::Get()->session_controller()->GetPrimaryUserPrefService()->SetBoolean(
       chromeos::assistant::prefs::kAssistantLaunchWithMicOpen, value);
+
+  // Ensure the value has taken effect.
+  ASSERT_EQ(GetAssistantState()->launch_with_mic_open(), value)
+      << "Changing this preference did not take effect immediately, which will "
+         "cause timing issues in this test. If this trace is seen we must add "
+         "a waiter here to wait for the new state to take effect.";
+}
+
+AssistantState* AssistantTestApiImpl::GetAssistantState() {
+  return AssistantState::Get();
 }
 
 void AssistantTestApiImpl::EnableAnimations() {
@@ -112,18 +138,29 @@ void AssistantTestApiImpl::EnableAnimations() {
   AppListView::SetShortAnimationForTesting(false);
 }
 
+bool AssistantTestApiImpl::AppListViewsHaveBeenCreated() const {
+  return contents_view_or_null() != nullptr;
+}
+
 ContentsView* AssistantTestApiImpl::contents_view() {
+  ContentsView* result = contents_view_or_null();
+  DCHECK(result) << "App list has not been initialized yet. "
+                    "Be sure to display the Assistant UI first.";
+  return result;
+}
+
+ContentsView* AssistantTestApiImpl::contents_view_or_null() const {
   auto* app_list_view =
       Shell::Get()->app_list_controller()->presenter()->GetView();
 
-  DCHECK(app_list_view) << "AppListView has not been initialized yet. "
-                           "Be sure to display the Assistant UI first.";
+  if (!app_list_view)
+    return nullptr;
 
   return app_list_view->app_list_main_view()->contents_view();
 }
 
 void AssistantTestApiImpl::SendKeyPress(ui::KeyboardCode key) {
-  ui::test::EventGenerator event_generator(window()->GetRootWindow());
+  ui::test::EventGenerator event_generator(root_window());
   event_generator.PressKey(key, /*flags=*/ui::EF_NONE);
 }
 

ash/app_list/views/assistant/assistant_test_api_impl.h

@@ -25,19 +25,17 @@ class AssistantTestApiImpl : public AssistantTestApi {
   AssistantTestApiImpl();
   ~AssistantTestApiImpl() override;
 
-  // AssistantTestApi:
+  // AssistantTestApi overrides:
   void DisableAnimations() override;
-
   bool IsVisible() override;
-
   void SendTextQuery(const std::string& query) override;
-
-  void EnableAssistant() override;
+  void SetAssistantEnabled(bool enable) override;
   void SetTabletMode(bool enable) override;
   void SetPreferVoice(bool value) override;
-
+  AssistantState* GetAssistantState() override;
   views::View* page_view() override;
   views::View* main_view() override;
+  views::View* ui_element_container() override;
   views::Textfield* input_text_field() override;
   views::View* mic_view() override;
   views::View* greeting_label() override;
@@ -45,11 +43,14 @@ class AssistantTestApiImpl : public AssistantTestApi {
   views::View* keyboard_input_toggle() override;
   aura::Window* window() override;
   views::View* app_list_view() override;
+  aura::Window* root_window() override;
 
  private:
   void EnableAnimations();
 
+  bool AppListViewsHaveBeenCreated() const;
   ContentsView* contents_view();
+  ContentsView* contents_view_or_null() const;
 
   void SendKeyPress(ui::KeyboardCode key);
 

ash/assistant/assistant_interaction_controller.cc

@@ -43,9 +43,9 @@ namespace ash {
 
 namespace {
 
-constexpr int kWarmerWelcomesMaxTimesTriggered = 3;
 constexpr char kAndroidIntentScheme[] = "intent://";
 constexpr char kAndroidIntentPrefix[] = "#Intent";
+using assistant::ui::kWarmerWelcomesMaxTimesTriggered;
 
 // Helpers ---------------------------------------------------------------------
 

ash/assistant/test/assistant_ash_test_base.cc

@@ -74,7 +74,7 @@ void AssistantAshTestBase::SetUp() {
   UpdateDisplay("1024x768");
 
   // Enable Assistant in settings.
-  test_api_->EnableAssistant();
+  test_api_->SetAssistantEnabled(true);
 
   // Cache controller.
   controller_ = Shell::Get()->assistant_controller();
@@ -82,7 +82,8 @@ void AssistantAshTestBase::SetUp() {
 
   // At this point our Assistant service is ready for use.
   // Indicate this by changing status from NOT_READY to READY.
-  AssistantState::Get()->NotifyStatusChanged(mojom::AssistantState::READY);
+  test_api_->GetAssistantState()->NotifyStatusChanged(
+      mojom::AssistantState::READY);
 
   test_api_->DisableAnimations();
 

ash/assistant/ui/assistant_ui_constants.h

@@ -46,6 +46,8 @@ extern const aura::WindowProperty<bool>* const kOnlyAllowMouseClickEvents;
 COMPONENT_EXPORT(ASSISTANT_UI_CONSTANTS)
 const gfx::FontList& GetDefaultFontList();
 
+constexpr int kWarmerWelcomesMaxTimesTriggered = 3;
+
 }  // namespace ui
 }  // namespace assistant
 

ash/assistant/ui/main_stage/assistant_card_element_view.cc

@@ -58,17 +58,16 @@ void CreateAndSendMouseClick(aura::WindowTreeHost* host,
 AssistantCardElementView::AssistantCardElementView(
     AssistantViewDelegate* delegate,
     const AssistantCardElement* card_element)
-    : delegate_(delegate),
-      contents_(const_cast<AssistantCardElement*>(card_element)->contents()) {
+    : delegate_(delegate), card_element_(card_element) {
   InitLayout(card_element);
 
-  // We observe |contents_| to receive events pertaining to the underlying web
+  // We observe contents() to receive events pertaining to the underlying web
   // contents including auto-resize and suppressed navigation events.
-  contents_->AddObserver(this);
+  contents()->AddObserver(this);
 }
 
 AssistantCardElementView::~AssistantCardElementView() {
-  contents_->RemoveObserver(this);
+  contents()->RemoveObserver(this);
 }
 
 const char* AssistantCardElementView::GetClassName() const {
@@ -99,11 +98,11 @@ void AssistantCardElementView::AboutToRequestFocusFromTabTraversal(
     bool reverse) {
   // Focus in the web contents will be reset in FocusThroughTabTraversal().
   focused_node_rect_ = gfx::Rect();
-  contents_->FocusThroughTabTraversal(reverse);
+  contents()->FocusThroughTabTraversal(reverse);
 }
 
 void AssistantCardElementView::OnFocus() {
-  contents_->Focus();
+  contents()->Focus();
 }
 
 void AssistantCardElementView::OnGestureEvent(ui::GestureEvent* event) {
@@ -168,7 +167,7 @@ void AssistantCardElementView::ScrollRectToVisible(const gfx::Rect& rect) {
 }
 
 void AssistantCardElementView::DidAutoResizeView(const gfx::Size& new_size) {
-  contents_->GetView()->view()->SetPreferredSize(new_size);
+  contents()->GetView()->view()->SetPreferredSize(new_size);
 }
 
 void AssistantCardElementView::DidSuppressNavigation(
@@ -217,10 +216,14 @@ void AssistantCardElementView::InitLayout(
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
   // Contents view.
-  AddChildView(contents_->GetView()->view());
+  AddChildView(contents()->GetView()->view());
 
   // OverrideDescription() doesn't work. Only names are read automatically.
   GetViewAccessibility().OverrideName(card_element->fallback());
 }
 
+content::NavigableContents* AssistantCardElementView::contents() {
+  return const_cast<AssistantCardElement*>(card_element_)->contents();
+}
+
 }  // namespace ash

ash/assistant/ui/main_stage/assistant_card_element_view.h

@@ -47,15 +47,20 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantCardElementView
   // contents. When animating AssistantCardElementView, we should animate the
   // layer for the native view as opposed to painting to and animating a layer
   // belonging to AssistantCardElementView.
-  gfx::NativeView native_view() { return contents_->GetView()->native_view(); }
+  gfx::NativeView native_view() { return contents()->GetView()->native_view(); }
+
+  const AssistantCardElement* GetCardElementForTesting() const {
+    return card_element_;
+  }
 
  private:
   void InitLayout(const AssistantCardElement* card_element);
 
+  content::NavigableContents* contents();
+
   AssistantViewDelegate* const delegate_;
 
-  // Owned by AssistantCardElement.
-  content::NavigableContents* const contents_;
+  const AssistantCardElement* const card_element_;
 
   // Rect of the focused node in the |contents_|.
   gfx::Rect focused_node_rect_;

ash/public/cpp/test/assistant_test_api.h

@@ -21,6 +21,8 @@ class View;
 
 namespace ash {
 
+class AssistantState;
+
 // Public test API for the Assistant UI.
 // This API works both for ash and browser tests.
 class ASH_EXPORT AssistantTestApi {
@@ -39,15 +41,20 @@ class ASH_EXPORT AssistantTestApi {
   // queries, i.e. the input text field must be visible and focussed.
   virtual void SendTextQuery(const std::string& query) = 0;
 
-  // Enables Assistant in settings.
-  virtual void EnableAssistant() = 0;
+  // Enables/Disables Assistant in settings.
+  // This will ensure the new value is propagated to the |AssistantState|.
+  virtual void SetAssistantEnabled(bool enabled) = 0;
 
   virtual void SetTabletMode(bool enable) = 0;
 
   // Changes the user setting controlling whether the user prefers voice or
-  // keyboard.
+  // keyboard (internally called |kAssistantLaunchWithMicOpen|).
+  // This will ensure the new value is propagated to the |AssistantState|.
   virtual void SetPreferVoice(bool value) = 0;
 
+  // Returns the interface to get/set Assistant related prefs and states.
+  virtual AssistantState* GetAssistantState() = 0;
+
   // Returns the top-level Assistant specific view.
   // Can only be used after the Assistant UI has been shown at least once.
   virtual views::View* page_view() = 0;
@@ -56,6 +63,11 @@ class ASH_EXPORT AssistantTestApi {
   // Can only be used after the Assistant UI has been shown at least once.
   virtual views::View* main_view() = 0;
 
+  // Returns the Assistant UI element container view, which contains all the
+  // responses.
+  // Can only be used after the Assistant UI has been shown at least once.
+  virtual views::View* ui_element_container() = 0;
+
   // Returns the text field used for inputting new queries.
   // Can only be used after the Assistant UI has been shown at least once.
   virtual views::Textfield* input_text_field() = 0;
@@ -78,11 +90,16 @@ class ASH_EXPORT AssistantTestApi {
 
   // Returns the window containing the Assistant UI.
   // Note that this window is shared for all components of the |AppList|.
+  // Can only be used after the Assistant UI has been shown at least once.
   virtual aura::Window* window() = 0;
 
   // Returns the app list view hosting the Assistant UI.
   // Can only be used after the Assistant UI has been shown at least once.
   virtual views::View* app_list_view() = 0;
+
+  // Returns the root window containing the Assistant UI (and the Ash shell).
+  // This can be used even when the Assistant UI has never been shown.
+  virtual aura::Window* root_window() = 0;
 };
 
 }  // namespace ash

chrome/browser/ui/ash/DEPS

@@ -14,6 +14,10 @@ specific_include_rules = {
    "+ash/components/strings/grit/ash_components_strings.h",
    "+ash/keyboard/ui",
    "+ash/public",
+   "+ash/assistant/model/ui/assistant_card_element.h",
+   "+ash/assistant/ui/assistant_ui_constants.h",
+   "+ash/assistant/ui/main_stage/assistant_card_element_view.h",
+   "+ash/assistant/ui/main_stage/assistant_text_element_view.h",
   ],
   # AshShellInit supports classic (non-mash) mode so allow ash/ includes.
   "ash_shell_init\.cc": [

chrome/browser/ui/ash/assistant/assistant_browsertest.cc

+// Copyright 2019 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 "chrome/browser/ui/ash/assistant/assistant_test_mixin.h"
+#include "chrome/test/base/mixin_based_in_process_browser_test.h"
+
+namespace chromeos {
+namespace assistant {
+
+class AssistantBrowserTest : public MixinBasedInProcessBrowserTest {
+ public:
+  AssistantBrowserTest() = default;
+  ~AssistantBrowserTest() override = default;
+
+  void ShowAssistantUi() {
+    if (!tester()->IsVisible())
+      tester()->PressAssistantKey();
+  }
+
+  AssistantTestMixin* tester() { return &tester_; }
+
+ private:
+  AssistantTestMixin tester_{&mixin_host_, this, embedded_test_server(),
+                             FakeS3Mode::kReplay};
+
+  DISALLOW_COPY_AND_ASSIGN(AssistantBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(AssistantBrowserTest,
+                       ShouldOpenAssistantUiWhenPressingAssistantKey) {
+  tester()->StartAssistantAndWaitForReady();
+
+  tester()->PressAssistantKey();
+
+  EXPECT_TRUE(tester()->IsVisible());
+}
+
+IN_PROC_BROWSER_TEST_F(AssistantBrowserTest, ShouldDisplayTextResponse) {
+  tester()->StartAssistantAndWaitForReady();
+
+  ShowAssistantUi();
+
+  tester()->SendTextQuery("test");
+  tester()->ExpectAnyOfTheseTextResponses({
+      "No one told me there would be a test",
+      "You're coming in loud and clear",
+      "debug OK",
+      "I can assure you, this thing's on",
+      "Is this thing on?",
+  });
+}
+
+IN_PROC_BROWSER_TEST_F(AssistantBrowserTest, ShouldDisplayCardResponse) {
+  tester()->StartAssistantAndWaitForReady();
+
+  ShowAssistantUi();
+
+  EXPECT_TRUE(tester()->IsVisible());
+
+  tester()->SendTextQuery("What is the highest mountain in the world?");
+  tester()->ExpectCardResponse("Mount Everest");
+}
+
+}  // namespace assistant
+}  // namespace chromeos

chrome/browser/ui/ash/assistant/assistant_client.cc

@@ -27,7 +27,6 @@
 #include "content/public/browser/media_session_service.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/service_process_host.h"
-#include "content/public/common/content_switches.h"
 #include "services/identity/public/mojom/identity_service.mojom.h"
 
 namespace {
@@ -82,12 +81,10 @@ void AssistantClient::MaybeInit(Profile* profile) {
 
   initialized_ = true;
 
-  bool is_test = base::CommandLine::ForCurrentProcess()->HasSwitch(
-      ::switches::kBrowserTest);
   auto* service =
       AssistantServiceConnection::GetForProfile(profile_)->service();
   service->Init(client_receiver_.BindNewPipeAndPassRemote(),
-                device_actions_.AddReceiver(), is_test);
+                device_actions_.AddReceiver());
   assistant_image_downloader_ = std::make_unique<AssistantImageDownloader>();
   assistant_setup_ = std::make_unique<AssistantSetup>(service);
 

chrome/browser/ui/ash/assistant/assistant_test_mixin.h

+// Copyright 2019 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 CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_TEST_MIXIN_H_
+#define CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_TEST_MIXIN_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/time/time.h"
+#include "chrome/browser/ui/ash/assistant/test/fake_s3_server.h"
+#include "chrome/test/base/mixin_based_in_process_browser_test.h"
+#include "ui/events/keycodes/keyboard_codes_posix.h"
+
+class PrefService;
+
+namespace ash {
+class AssistantTestApi;
+}  // namespace ash
+
+namespace chromeos {
+namespace assistant {
+
+class FakeS3Server;
+class LoggedInUserMixin;
+
+// Default wait time before we conclude the wait actions have timed out.
+constexpr base::TimeDelta kDefaultWaitTimeout =
+    base::TimeDelta::FromSeconds(10);
+
+// Creates everything required to test the Assistant in browser tests.
+// This includes:
+//     - Installing a fake Gaia server so the test user is able to use the
+//       Assistant.
+//     - Setting up a fake S3 server to spoof fake interactions.
+//     - Enabling the Assistant service.
+//     - Disabling all Assistant animations.
+//
+// See definition of |chromeos::assistant::FakeS3Server| for an explanation of
+// the different modes the fake S3 server can run in (specified by passing
+// |FakeS3Mode| into the constructor).
+class AssistantTestMixin : public InProcessBrowserTestMixin {
+ public:
+  AssistantTestMixin(InProcessBrowserTestMixinHost* host,
+                     InProcessBrowserTest* test_base,
+                     net::EmbeddedTestServer* embedded_test_server,
+                     FakeS3Mode mode);
+  ~AssistantTestMixin() override;
+
+  // InProcessBrowserTestMixin overrides:
+  void SetUpCommandLine(base::CommandLine* command_line) override;
+  void SetUpOnMainThread() override;
+  void TearDownOnMainThread() override;
+
+  // Starts the Assistant service and wait until it is ready to process
+  // queries. Should be called as the first action in every test.
+  void StartAssistantAndWaitForReady(
+      base::TimeDelta wait_timeout = kDefaultWaitTimeout);
+
+  // Changes the user setting controlling whether the user prefers voice or
+  // keyboard.
+  void SetPreferVoice(bool prefer_voice);
+
+  // Submits a text query. Can only be used when the Assistant UI is visible and
+  // displaying the query input text field.
+  void SendTextQuery(const std::string& query);
+
+  // Waits until a card response is rendered that contains the given text.
+  // If |expected_response| is not received in |wait_timeout|, this will fail
+  // the test.
+  void ExpectCardResponse(const std::string& expected_response,
+                          base::TimeDelta wait_timeout = kDefaultWaitTimeout);
+
+  // Waits until a text response is rendered that contains the given text.
+  // If |expected_response| is not received in |wait_timeout|, this will fail
+  // the test.
+  void ExpectTextResponse(const std::string& expected_response,
+                          base::TimeDelta wait_timeout = kDefaultWaitTimeout);
+
+  // Same as above but checks if any of the given responses are encountered.
+  void ExpectAnyOfTheseTextResponses(
+      const std::vector<std::string>& expected_responses,
+      base::TimeDelta wait_timeout = kDefaultWaitTimeout);
+
+  // Presses the Assistant key, which will toggle the Assistant UI.
+  void PressAssistantKey();
+
+  // Returns true if the Assistant UI is currently visible.
+  bool IsVisible();
+
+ private:
+  PrefService* GetUserPreferences();
+  void SendKeyPress(ui::KeyboardCode key);
+  void DisableAssistant();
+  void DisableWarmerWelcome();
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+  FakeS3Server fake_s3_server_;
+  FakeS3Mode mode_;
+  std::unique_ptr<ash::AssistantTestApi> test_api_;
+  std::unique_ptr<LoggedInUserMixin> user_mixin_;
+
+  DISALLOW_COPY_AND_ASSIGN(AssistantTestMixin);
+};
+
+}  // namespace assistant
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_UI_ASH_ASSISTANT_ASSISTANT_TEST_MIXIN_H_

chrome/browser/ui/ash/assistant/test/fake_s3_server.cc

+// Copyright 2019 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 "chrome/browser/ui/ash/assistant/test/fake_s3_server.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
+#include "base/process/launch.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "chromeos/assistant/internal/internal_constants.h"
+#include "chromeos/services/assistant/service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace assistant {
+
+namespace {
+
+// Folder where the S3 communications are stored when running in replay mode.
+constexpr const char kTestDataFolder[] =
+    "chromeos/assistant/internal/test_data/";
+
+base::FilePath GetExecutableDir() {
+  base::FilePath result;
+  base::PathService::Get(base::DIR_EXE, &result);
+  return result;
+}
+
+base::FilePath GetSourceDir() {
+  base::FilePath result;
+  base::PathService::Get(base::DIR_SOURCE_ROOT, &result);
+  return result;
+}
+
+std::string GetSanitizedTestName() {
+  std::string test_name = base::ToLowerASCII(base::StringPrintf(
+      "%s_%s",
+      testing::UnitTest::GetInstance()->current_test_info()->test_case_name(),
+      testing::UnitTest::GetInstance()->current_test_info()->name()));
+  std::string new_test_name;
+  base::ReplaceChars(test_name, "/", "_", &new_test_name);
+  return new_test_name;
+}
+
+std::string GetTestDataFileName() {
+  return GetSourceDir()
+      .Append(FILE_PATH_LITERAL(kTestDataFolder))
+      .Append(FILE_PATH_LITERAL(GetSanitizedTestName() + ".fake_s3.proto"))
+      .MaybeAsASCII();
+}
+
+const std::string GetAccessTokenFromEnvironmentOrDie() {
+  const char* token = std::getenv("TOKEN");
+  CHECK(token && strlen(token))
+      << "No token found in the environmental variable $TOKEN.\n"
+      << kGenerateTokenInstructions;
+  return token;
+}
+
+std::string FakeS3ModeToString(FakeS3Mode mode) {
+  switch (mode) {
+    case FakeS3Mode::kProxy:
+      return "PROXY";
+    case FakeS3Mode::kRecord:
+      return "RECORD";
+    case FakeS3Mode::kReplay:
+      return "REPLAY";
+  }
+  NOTREACHED();
+}
+
+void AppendArgument(base::CommandLine* command_line,
+                    const std::string& name,
+                    const std::string& value) {
+  // Note we can't use |AppendSwitchASCII| as that will add "<name>=<value>",
+  // and the fake s3 server binary does not support '='.
+  command_line->AppendArg(name);
+  command_line->AppendArg(value);
+}
+
+}  // namespace
+
+// Selects a port for the fake S3 server to use.
+// This will use a file-based lock because different test shards might be trying
+// to run fake S3 servers at the same time, and we need to ensure they use
+// different ports.
+class PortSelector {
+ public:
+  PortSelector() { SelectPort(); }
+  PortSelector(PortSelector&) = delete;
+  PortSelector& operator=(PortSelector&) = delete;
+  ~PortSelector() {
+    lock_file_.Close();
+    base::DeleteFileRecursively(GetLockFilePath());
+  }
+
+  int port() const { return port_; }
+
+ private:
+  // The first port we'll try to use. Randomly chosen to be outside of the range
+  // of known ports.
+  constexpr static int kStartPort = 23600;
+  // Maximum number of ports we'll try before we give up and conclude no ports
+  // are available (which really should not happen).
+  constexpr static int kMaxAttempts = 20000;
+
+  void SelectPort() {
+    for (int offset = 0; offset < kMaxAttempts; offset++) {
+      port_ = kStartPort + offset;
+      lock_file_ = base::File(GetLockFilePath(), GetFileFlags());
+      if (lock_file_.IsValid())
+        return;
+    }
+    CHECK(false) << "Failed to find an available port.";
+  }
+
+  base::FilePath GetLockFilePath() const {
+    std::string file_name = "port_" + base::NumberToString(port_) + "_lock";
+    return GetLockFileDirectory().Append(file_name);
+  }
+  static base::FilePath GetLockFileDirectory() {
+    base::FilePath result;
+    bool success = base::GetTempDir(&result);
+    EXPECT_TRUE(success);
+    return result;
+  }
+
+  static int GetFileFlags() {
+    return base::File::FLAG_CREATE | base::File::FLAG_EXCLUSIVE_WRITE |
+           base::File::FLAG_WRITE;
+  }
+
+  // File exclusively opened on the file-system, to ensure no other fake S3
+  // server uses the same port.
+  base::File lock_file_;
+  int port_;
+};
+
+FakeS3Server::FakeS3Server()
+    : port_selector_(std::make_unique<PortSelector>()) {}
+
+FakeS3Server::~FakeS3Server() {
+  Teardown();
+}
+
+void FakeS3Server::Setup(FakeS3Mode mode) {
+  SetAccessTokenForMode(mode);
+  StartS3ServerProcess(mode);
+  SetFakeS3ServerURI();
+}
+
+void FakeS3Server::Teardown() {
+  StopS3ServerProcess();
+  UnsetFakeS3ServerURI();
+}
+
+std::string FakeS3Server::GetAccessToken() const {
+  return access_token_;
+}
+
+void FakeS3Server::SetAccessTokenForMode(FakeS3Mode mode) {
+  if (mode == FakeS3Mode::kProxy || mode == FakeS3Mode::kRecord) {
+    access_token_ = GetAccessTokenFromEnvironmentOrDie();
+  }
+}
+
+void FakeS3Server::SetFakeS3ServerURI() {
+  // Note this must be stored in a local variable, as
+  // |Service::OverrideS3ServerUriForTesting| does not take ownership of the
+  // |const char *|.
+  fake_s3_server_uri_ = "localhost:" + base::NumberToString(port());
+  Service::OverrideS3ServerUriForTesting(fake_s3_server_uri_.c_str());
+}
+
+void FakeS3Server::UnsetFakeS3ServerURI() {
+  Service::OverrideS3ServerUriForTesting(nullptr);
+  fake_s3_server_uri_ = "";
+}
+
+void FakeS3Server::StartS3ServerProcess(FakeS3Mode mode) {
+  base::FilePath fake_s3_server_main =
+      GetExecutableDir().Append(FILE_PATH_LITERAL(kFakeS3ServerBinary));
+
+  base::CommandLine command_line(fake_s3_server_main);
+  AppendArgument(&command_line, "--port", base::NumberToString(port()));
+  AppendArgument(&command_line, "--mode", FakeS3ModeToString(mode));
+  AppendArgument(&command_line, "--auth_token", GetAccessToken());
+  AppendArgument(&command_line, "--test_data_file", GetTestDataFileName());
+
+  fake_s3_server_ = base::LaunchProcess(command_line, base::LaunchOptions{});
+}
+
+void FakeS3Server::StopS3ServerProcess() {
+  fake_s3_server_.Terminate(/*exit_code=*/0, /*wait=*/true);
+}
+
+int FakeS3Server::port() const {
+  return port_selector_->port();
+}
+
+}  // namespace assistant
+}  // namespace chromeos

chrome/browser/ui/ash/assistant/test/fake_s3_server.h

+// Copyright 2019 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 CHROME_BROWSER_UI_ASH_ASSISTANT_TEST_FAKE_S3_SERVER_H_
+#define CHROME_BROWSER_UI_ASH_ASSISTANT_TEST_FAKE_S3_SERVER_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/process/process.h"
+
+namespace chromeos {
+namespace assistant {
+
+class PortSelector;
+
+enum class FakeS3Mode {
+  // In this mode all S3 requests are forwarded to the S3 server.
+  kProxy,
+  // In this mode all S3 requests are forwarded to the S3 server, and the
+  // responses are recorded.
+  kRecord,
+  // In this mode all S3 requests are handled by replaying the responses stored
+  // while running in |kRecord| mode.
+  kReplay,
+};
+
+// Class that starts/stops a fake S3 server.
+// Note that this will also ensure the Assistant service knows to use the fake
+// s3 server.
+//
+// A valid access token is required if mode is |kProxy| or |kReplay|. See
+// |kGenerateTokenInstructions| for information on how to get one.
+class FakeS3Server {
+ public:
+  FakeS3Server();
+  ~FakeS3Server();
+
+  // Starts the fake S3 server, and tells the Assistant service to use its URI
+  // for all S3 requests.
+  void Setup(FakeS3Mode mode);
+  void Teardown();
+
+  // Returns the access token used by the S3 Server. This is only populated
+  // after |Setup| is called.
+  std::string GetAccessToken() const;
+
+ private:
+  void SetAccessTokenForMode(FakeS3Mode mode);
+  void SetFakeS3ServerURI();
+  void UnsetFakeS3ServerURI();
+  void StartS3ServerProcess(FakeS3Mode mode);
+  void StopS3ServerProcess();
+
+  int port() const;
+
+  std::string access_token_{"FAKE_ACCESS_TOKEN"};
+  std::string fake_s3_server_uri_;
+
+  std::unique_ptr<PortSelector> port_selector_;
+
+  base::Process fake_s3_server_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeS3Server);
+};
+
+}  // namespace assistant
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_UI_ASH_ASSISTANT_TEST_FAKE_S3_SERVER_H_

chrome/test/BUILD.gn

@@ -15,6 +15,7 @@ import("//chrome/browser/buildflags.gni")
 import("//chrome/chrome_repack_locales.gni")
 import("//chrome/common/features.gni")
 import("//chrome/test/base/js2gtest.gni")
+import("//chromeos/assistant/assistant.gni")
 import("//components/feature_engagement/features.gni")
 import("//components/feed/features.gni")
 import("//components/gwp_asan/buildflags/buildflags.gni")
@@ -2443,6 +2444,22 @@ if (!is_android) {
         sources +=
             [ "../browser/chromeos/extensions/printing/printing_apitest.cc" ]
       }
+      if (enable_cros_libassistant) {
+        sources += [
+          "../browser/ui/ash/assistant/assistant_browsertest.cc",
+          "../browser/ui/ash/assistant/assistant_test_mixin.cc",
+          "../browser/ui/ash/assistant/assistant_test_mixin.h",
+          "../browser/ui/ash/assistant/test/fake_s3_server.cc",
+          "../browser/ui/ash/assistant/test/fake_s3_server.h",
+        ]
+
+        deps += [ "//chromeos/assistant/internal:internal" ]
+
+        data += [ "//chromeos/assistant/internal/test_data/" ]
+
+        data_deps +=
+            [ "//chromeos/assistant/internal:build_fake_s3_server_main" ]
+      }
       sources -= [
         "../../apps/load_and_launch_browsertest.cc",
         "../browser/policy/policy_startup_browsertest.cc",

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -151,6 +151,7 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl(
     std::unique_ptr<AssistantManagerServiceDelegate> delegate,
     std::unique_ptr<network::PendingSharedURLLoaderFactory>
         pending_url_loader_factory,
+    base::Optional<std::string> s3_server_uri_override,
     bool is_signed_out_mode)
     : client_(client),
       media_session_(std::make_unique<AssistantMediaSession>(client_, this)),
@@ -165,6 +166,7 @@ AssistantManagerServiceImpl::AssistantManagerServiceImpl(
       delegate_(std::move(delegate)),
       background_thread_("background thread"),
       is_signed_out_mode_(is_signed_out_mode),
+      libassistant_config_(CreateLibAssistantConfig(s3_server_uri_override)),
       weak_factory_(this) {
   background_thread_.Start();
 
@@ -1182,7 +1184,7 @@ void AssistantManagerServiceImpl::StartAssistantInternal(
       assistant::features::IsMediaSessionIntegrationEnabled());
 
   new_assistant_manager_ = delegate_->CreateAssistantManager(
-      platform_api_.get(), CreateLibAssistantConfig());
+      platform_api_.get(), libassistant_config_);
   new_assistant_manager_internal_ =
       delegate_->UnwrapAssistantManagerInternal(new_assistant_manager_.get());
 
@@ -1389,7 +1391,6 @@ void AssistantManagerServiceImpl::MediaSessionMetadataChanged(
   UpdateMediaState();
 }
 
-
 void AssistantManagerServiceImpl::OnPlaybackStateChange(
     const MediaStatus& status) {
   if (media_session_)

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -13,6 +13,7 @@
 #include "ash/public/cpp/ambient/ambient_mode_state.h"
 #include "ash/public/mojom/assistant_controller.mojom.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "chromeos/assistant/internal/action/cros_action_module.h"
@@ -105,6 +106,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
       std::unique_ptr<AssistantManagerServiceDelegate> delegate,
       std::unique_ptr<network::PendingSharedURLLoaderFactory>
           pending_url_loader_factory,
+      base::Optional<std::string> s3_server_uri_override,
       bool is_signed_out_mode);
 
   ~AssistantManagerServiceImpl() override;
@@ -379,6 +381,9 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
   base::UnguessableToken media_session_audio_focus_id_ =
       base::UnguessableToken::Null();
 
+  // Configuration passed to libassistant.
+  std::string libassistant_config_;
+
   mojo::Receiver<mojom::AppListEventSubscriber> app_list_subscriber_receiver_{
       this};
 

chromeos/services/assistant/assistant_manager_service_impl_unittest.cc

@@ -6,9 +6,11 @@
 
 #include <utility>
 
+#include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
+#include "base/values.h"
 #include "chromeos/assistant/internal/internal_util.h"
 #include "chromeos/assistant/internal/test_support/fake_assistant_manager.h"
 #include "chromeos/assistant/internal/test_support/fake_assistant_manager_internal.h"
@@ -38,6 +40,7 @@ using testing::StrictMock;
 using CommunicationErrorType = AssistantManagerService::CommunicationErrorType;
 
 namespace {
+
 // Return the list of all libassistant error codes that are considered to be
 // authentication errors. This list is created on demand as there is no clear
 // enum that defines these, and we don't want to hard code this list in the
@@ -184,12 +187,17 @@ class AssistantManagerServiceImplTest : public testing::Test {
         task_environment.GetMainThreadTaskRunner(), &assistant_state_,
         PowerManagerClient::Get());
 
+    CreateAssistantManagerServiceImpl(/*libassistant_config=*/{});
+  }
+
+  void CreateAssistantManagerServiceImpl(
+      base::Optional<std::string> s3_server_uri_override = base::nullopt) {
     auto delegate = std::make_unique<FakeAssistantManagerServiceDelegate>();
     delegate_ = delegate.get();
 
     assistant_manager_service_ = std::make_unique<AssistantManagerServiceImpl>(
         &assistant_client_, service_context_.get(), std::move(delegate),
-        shared_url_loader_factory_->Clone(),
+        shared_url_loader_factory_->Clone(), s3_server_uri_override,
         /*is_signed_out_mode=*/false);
   }
 
@@ -204,6 +212,8 @@ class AssistantManagerServiceImplTest : public testing::Test {
 
   ash::AssistantState* assistant_state() { return &assistant_state_; }
 
+  FakeAssistantManagerServiceDelegate* delegate() { return delegate_; }
+
   FakeAssistantManager* fake_assistant_manager() {
     return delegate_->assistant_manager();
   }
@@ -275,6 +285,20 @@ class AssistantManagerServiceImplTest : public testing::Test {
 
   DISALLOW_COPY_AND_ASSIGN(AssistantManagerServiceImplTest);
 };
+
+// Tests if the JSON string contains the given path with the given value
+#define EXPECT_HAS_PATH_WITH_VALUE(config_string, path, expected_value)    \
+  ({                                                                       \
+    base::Optional<base::Value> config =                                   \
+        base::JSONReader::Read(config_string);                             \
+    ASSERT_TRUE(config.has_value());                                       \
+    const base::Value* actual = config->FindPath(path);                    \
+    base::Value expected = base::Value(expected_value);                    \
+    ASSERT_NE(actual, nullptr)                                             \
+        << "Path '" << path << "' not found in config: " << config_string; \
+    EXPECT_EQ(*actual, expected);                                          \
+  })
+
 }  // namespace
 
 TEST_F(AssistantManagerServiceImplTest, StateShouldStartAsStopped) {
@@ -375,6 +399,17 @@ TEST_F(AssistantManagerServiceImplTest,
   EXPECT_EQ(kUserID, fake_assistant_manager()->user_id());
 }
 
+TEST_F(AssistantManagerServiceImplTest,
+       ShouldPassS3ServerUriOverrideToAssistantManager) {
+  CreateAssistantManagerServiceImpl("the-uri-override");
+
+  Start();
+  WaitUntilStartIsFinished();
+
+  EXPECT_HAS_PATH_WITH_VALUE(delegate()->libassistant_config(),
+                             "testing.s3_grpc_server_uri", "the-uri-override");
+}
+
 TEST_F(AssistantManagerServiceImplTest, ShouldPauseMediaManagerOnPause) {
   StrictMock<MockMediaManager> mock;
   fake_assistant_manager()->SetMediaManager(&mock);

chromeos/services/assistant/public/mojom/assistant.mojom

@@ -28,8 +28,7 @@ interface AssistantService {
   // Initiates assistant and provides interfaces for assistant to call into the
   // browser.
   Init(pending_remote<Client> assistant_client_interface,
-       pending_remote<DeviceActions> device_actions_interface,
-       bool is_test);
+       pending_remote<DeviceActions> device_actions_interface);
 
   // Binds the main assistant backend interface.
   BindAssistant(pending_receiver<Assistant> receiver);

chromeos/services/assistant/service.cc

@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/optional.h"
 #include "base/rand_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/time/time.h"
@@ -62,6 +63,8 @@ constexpr base::TimeDelta kMaxTokenRefreshDelay =
 
 // Testing override for the AssistantSettingsManager implementation.
 AssistantSettingsManager* g_settings_manager_override = nullptr;
+// Testing override for the URI used to contact the s3 server.
+const char* g_s3_server_uri_override = nullptr;
 
 ash::mojom::AssistantState ToAssistantStatus(
     AssistantManagerService::State state) {
@@ -79,6 +82,14 @@ ash::mojom::AssistantState ToAssistantStatus(
   }
 }
 
+#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
+base::Optional<std::string> GetS3ServerUriOverride() {
+  if (g_s3_server_uri_override)
+    return g_s3_server_uri_override;
+  return base::nullopt;
+}
+#endif
+
 }  // namespace
 
 class Service::Context : public ServiceContext {
@@ -167,6 +178,11 @@ void Service::OverrideSettingsManagerForTesting(
   g_settings_manager_override = manager;
 }
 
+// static
+void Service::OverrideS3ServerUriForTesting(const char* uri) {
+  g_s3_server_uri_override = uri;
+}
+
 void Service::SetIdentityAccessorForTesting(
     mojo::PendingRemote<identity::mojom::IdentityAccessor> identity_accessor) {
   identity_accessor_.Bind(std::move(identity_accessor));
@@ -176,11 +192,19 @@ void Service::SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer) {
   token_refresh_timer_ = std::move(timer);
 }
 
+void Service::SetAssistantManagerServiceForTesting(
+    std::unique_ptr<AssistantManagerService> assistant_manager_service) {
+  DCHECK(assistant_manager_service_ == nullptr);
+  assistant_manager_service_for_testing_ = std::move(assistant_manager_service);
+}
+
+AssistantStateProxy* Service::GetAssistantStateProxyForTesting() {
+  return &assistant_state_;
+}
+
 void Service::Init(mojo::PendingRemote<mojom::Client> client,
-                   mojo::PendingRemote<mojom::DeviceActions> device_actions,
-                   bool is_test) {
+                   mojo::PendingRemote<mojom::DeviceActions> device_actions) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  is_test_ = is_test;
   client_.Bind(std::move(client));
   device_actions_.Bind(std::move(device_actions));
 
@@ -416,7 +440,8 @@ void Service::GetPrimaryAccountInfoCallback(
   scopes.insert(kScopeAuthGcm);
   if (features::IsClearCutLogEnabled())
     scopes.insert(kScopeClearCutLog);
-  identity_accessor_->GetAccessToken(
+
+  GetIdentityAccessor()->GetAccessToken(
       *account_id, scopes, "cros_assistant",
       base::BindOnce(&Service::GetAccessTokenCallback, base::Unretained(this)));
 }
@@ -462,12 +487,10 @@ void Service::CreateAssistantManagerService() {
 
 std::unique_ptr<AssistantManagerService>
 Service::CreateAndReturnAssistantManagerService() {
-#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
-  if (is_test_) {
-    // Use fake service in browser tests.
-    return std::make_unique<FakeAssistantManagerServiceImpl>();
-  }
+  if (assistant_manager_service_for_testing_)
+    return std::move(assistant_manager_service_for_testing_);
 
+#if BUILDFLAG(ENABLE_CROS_LIBASSISTANT)
   DCHECK(client_);
 
   mojo::PendingRemote<device::mojom::BatteryMonitor> battery_monitor;
@@ -481,7 +504,8 @@ Service::CreateAndReturnAssistantManagerService() {
   DCHECK(pending_url_loader_factory_);
   return std::make_unique<AssistantManagerServiceImpl>(
       client_.get(), context(), std::move(delegate),
-      std::move(pending_url_loader_factory_), is_signed_out_mode_);
+      std::move(pending_url_loader_factory_), GetS3ServerUriOverride(),
+      is_signed_out_mode_);
 #else
   return std::make_unique<FakeAssistantManagerServiceImpl>();
 #endif

chromeos/services/assistant/service.h

@@ -75,12 +75,26 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
   // Allows tests to override the AssistantSettingsManager bound by the service.
   static void OverrideSettingsManagerForTesting(
       AssistantSettingsManager* manager);
+  // Allows tests to override the S3 server URI used by the service.
+  // The caller must ensure the memory passed in remains valid.
+  // This override can be removed by passing in a nullptr.
+  // Note: This would look nicer if it was a class method and not static,
+  // but unfortunately this must be called before |Service| tries to create the
+  // |AssistantManagerService|, which happens really soon after the service
+  // itself is created, so we do not have time in our tests to grab a handle
+  // to |Service| and set this before it is too late.
+  static void OverrideS3ServerUriForTesting(const char* uri);
 
   void SetIdentityAccessorForTesting(
       mojo::PendingRemote<identity::mojom::IdentityAccessor> identity_accessor);
 
   void SetTimerForTesting(std::unique_ptr<base::OneShotTimer> timer);
 
+  void SetAssistantManagerServiceForTesting(
+      std::unique_ptr<AssistantManagerService> assistant_manager_service);
+
+  AssistantStateProxy* GetAssistantStateProxyForTesting();
+
  private:
   friend class AssistantServiceTest;
 
@@ -88,8 +102,7 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
 
   // mojom::AssistantService overrides
   void Init(mojo::PendingRemote<mojom::Client> client,
-            mojo::PendingRemote<mojom::DeviceActions> device_actions,
-            bool is_test) override;
+            mojo::PendingRemote<mojom::DeviceActions> device_actions) override;
   void BindAssistant(mojo::PendingReceiver<mojom::Assistant> receiver) override;
   void BindSettingsManager(
       mojo::PendingReceiver<mojom::AssistantSettingsManager> receiver) override;
@@ -172,8 +185,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
                  chromeos::PowerManagerClient::Observer>
       power_manager_observer_{this};
 
-  // Whether running inside a test environment.
-  bool is_test_ = false;
   // Whether the current user session is active.
   bool session_active_ = false;
   // Whether the lock screen is on.
@@ -184,6 +195,12 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) Service
   // using user's signed in account information.
   bool is_signed_out_mode_ = false;
 
+  // The value passed into |SetAssistantManagerServiceForTesting|.
+  // Will be moved into |assistant_manager_service_| when the service is
+  // supposed to be created.
+  std::unique_ptr<AssistantManagerService>
+      assistant_manager_service_for_testing_ = nullptr;
+
   base::Optional<std::string> access_token_;
 
   mojo::Remote<mojom::AssistantController> assistant_controller_;

chromeos/services/assistant/service_unittest.cc

@@ -184,7 +184,8 @@ class AssistantServiceTest : public testing::Test {
     service_ = std::make_unique<Service>(
         remote_service_.BindNewPipeAndPassReceiver(),
         shared_url_loader_factory_->Clone(), &pref_service_);
-    service_->is_test_ = true;
+    service_->SetAssistantManagerServiceForTesting(
+        std::make_unique<FakeAssistantManagerServiceImpl>());
 
     mock_task_runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
         base::Time::Now(), base::TimeTicks::Now());
@@ -197,8 +198,7 @@ class AssistantServiceTest : public testing::Test {
         fake_identity_accessor_.CreatePendingRemoteAndBind());
 
     remote_service_->Init(fake_assistant_client_.MakeRemote(),
-                          fake_device_actions_.CreatePendingRemoteAndBind(),
-                          /*is_test=*/true);
+                          fake_device_actions_.CreatePendingRemoteAndBind());
     base::RunLoop().RunUntilIdle();
   }
 

chromeos/services/assistant/test_support/fake_assistant_manager_service_delegate.cc

@@ -30,8 +30,9 @@ FakeAssistantManagerServiceDelegate::CreatePlatformApi(
 std::unique_ptr<assistant_client::AssistantManager>
 FakeAssistantManagerServiceDelegate::CreateAssistantManager(
     assistant_client::PlatformApi* platform_api,
-    const std::string& lib_assistant_config) {
+    const std::string& libassistant_config) {
   DCHECK(assistant_manager_);
+  libassistant_config_ = libassistant_config;
   return std::move(assistant_manager_);
 }
 

chromeos/services/assistant/test_support/fake_assistant_manager_service_delegate.h

@@ -34,10 +34,12 @@ class FakeAssistantManagerServiceDelegate
       override;
   std::unique_ptr<assistant_client::AssistantManager> CreateAssistantManager(
       assistant_client::PlatformApi* platform_api,
-      const std::string& lib_assistant_config) override;
+      const std::string& libassistant_config) override;
   assistant_client::AssistantManagerInternal* UnwrapAssistantManagerInternal(
       assistant_client::AssistantManager* assistant_manager) override;
 
+  std::string libassistant_config() const { return libassistant_config_; }
+
  private:
   // Will be initialized in the constructor and moved out when
   // |CreateAssistantManager| is called.
@@ -45,6 +47,9 @@ class FakeAssistantManagerServiceDelegate
   std::unique_ptr<FakeAssistantManagerInternal> assistant_manager_internal_;
   FakeAssistantManager* assistant_manager_ptr_;
 
+  // Config passed to LibAssistant when it was started.
+  std::string libassistant_config_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeAssistantManagerServiceDelegate);
 };
 

chromeos/services/assistant/utils.cc

@@ -51,7 +51,8 @@ base::FilePath GetRootPath() {
   return home_dir;
 }
 
-std::string CreateLibAssistantConfig() {
+std::string CreateLibAssistantConfig(
+    base::Optional<std::string> s3_server_uri_override) {
   using Value = base::Value;
   using Type = base::Value::Type;
 
@@ -124,6 +125,18 @@ std::string CreateLibAssistantConfig() {
 
   config.SetKey("audio_input", std::move(audio_input));
 
+  // Use http unless we're using the fake s3 server, which requires grpc.
+  if (s3_server_uri_override)
+    config.SetStringPath("internal.transport_type", "GRPC");
+  else
+    config.SetStringPath("internal.transport_type", "HTTP");
+
+  // Finally add in the server uri override.
+  if (s3_server_uri_override) {
+    config.SetStringPath("testing.s3_grpc_server_uri",
+                         s3_server_uri_override.value());
+  }
+
   std::string json;
   base::JSONWriter::Write(config, &json);
   return json;

chromeos/services/assistant/utils.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/macros.h"
+#include "base/optional.h"
 
 namespace base {
 class FilePath;
@@ -18,7 +19,9 @@ namespace assistant {
 
 base::FilePath GetRootPath();
 
-std::string CreateLibAssistantConfig();
+// Creates the configuration for libassistant.
+std::string CreateLibAssistantConfig(
+    base::Optional<std::string> s3_server_uri_override = base::nullopt);
 
 }  // namespace assistant
 }  // namespace chromeos