Reland "Increase coverage for EULA browser tests"

This is a reland of 426df2be

Original change's description:
> Increase coverage for EULA browser tests
>
> Adds tests for:
> * Clicking on "System security settings" button in EULA screen
> * Accepting and rejecting usage stats collection
> * Clicking on "Learn more" button in EULA screen
>
> Removes dead code from EulaScreen class.
>
> Adds a HelpApp extension manifest to use in tests and modifies
> HelpAppLauncher class to allow tests to set the extension id.
>
> Adds a HelpAppTestHelper class to facilitate writing tests that
> open HelpApp dialogs.
>
> Change-Id: Id7841be5bf3faabb067d3a75d3b034bdf0e00e32
> Reviewed-on:
https://chromium-review.googlesource.com/c/chromium/src/+/1554123
> Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
> Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
> Reviewed-by: Toni Baržić <tbarzic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#648759}

TBR=hashimoto

Bug: 947358
Change-Id: I23865c1957f300f55812f3275e830772aa27c32d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1560146Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649258}

chrome/browser/chromeos/BUILD.gn

@@ -2151,6 +2151,8 @@ static_library("test_support") {
     "login/screens/mock_update_screen.h",
     "login/screens/mock_welcome_screen.cc",
     "login/screens/mock_welcome_screen.h",
+    "login/test/help_app_test_helper.cc",
+    "login/test/help_app_test_helper.h",
     "login/test/test_condition_waiter.h",
     "login/test/test_predicate_waiter.cc",
     "login/test/test_predicate_waiter.h",

chrome/browser/chromeos/login/eula_browsertest.cc

@@ -8,17 +8,29 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/task/post_task.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/login/test/help_app_test_helper.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
+#include "chrome/browser/chromeos/settings/stats_reporting_controller.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/webui/chromeos/login/eula_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
+#include "chrome/installer/util/google_update_settings.h"
+#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
 #include "components/guest_view/browser/guest_view_manager.h"
+#include "components/metrics/metrics_pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_ui.h"
@@ -38,7 +50,6 @@ namespace {
 
 constexpr char kFakeOnlineEulaPath[] = "/intl/en-US/chrome/eula_text.html";
 constexpr char kFakeOnlineEula[] = "No obligations at all";
-
 #if defined(GOOGLE_CHROME_BUILD)
 // See IDS_ABOUT_TERMS_OF_SERVICE for the complete text.
 constexpr char kOfflineEULAWarning[] = "Chrome OS Terms";
@@ -153,6 +164,46 @@ class EulaTest : public OobeBaseTest {
     return *frame_set.begin();
   }
 
+  // Returns an Oobe JSChecker that sends 'click' events instead of 'tap'
+  // events when interacting with UI elements.
+  test::JSChecker NonPolymerOobeJS() {
+    test::JSChecker js = test::OobeJS();
+    js.set_polymer_ui(false);
+    return js;
+  }
+
+  base::OnceClosure SetCollectStatsConsentClosure(bool consented) {
+    return base::BindOnce(
+        base::IgnoreResult(&GoogleUpdateSettings::SetCollectStatsConsent),
+        consented);
+  }
+
+  // Waits until |blocking_closure| finishes executing.
+  void WaitForBlockingCall(base::OnceClosure blocking_closure) {
+    base::RunLoop runloop;
+    base::PostTaskWithTraitsAndReply(FROM_HERE, {base::MayBlock()},
+                                     std::move(blocking_closure),
+                                     runloop.QuitClosure());
+    runloop.Run();
+  }
+
+  // Waits and returns the result of evaluating |blocking_closure|.
+  bool EvaluateBlocking(base::OnceCallback<bool()> blocking_closure) {
+    bool result = false;
+    base::RunLoop runloop;
+    base::PostTaskWithTraitsAndReplyWithResult(
+        FROM_HERE, {base::MayBlock()}, std::move(blocking_closure),
+        base::BindOnce(
+            [](base::RepeatingClosure quit_closure, bool* result_out,
+               bool evaluation_result) {
+              *result_out = evaluation_result;
+              quit_closure.Run();
+            },
+            runloop.QuitClosure(), &result));
+    runloop.Run();
+    return result;
+  }
+
  private:
   std::unique_ptr<HttpResponse> HandleRequest(const HttpRequest& request) {
     GURL request_url = GURL("http://localhost").Resolve(request.relative_url);
@@ -211,5 +262,113 @@ IN_PROC_BROWSER_TEST_F(EulaTest, LoadOffline) {
               std::string::npos);
 }
 
+// Tests that clicking on "System security settings" button opens a dialog
+// showing the TPM password.
+IN_PROC_BROWSER_TEST_F(EulaTest, DisplaysTpmPassword) {
+  ShowEulaScreen();
+
+  NonPolymerOobeJS().TapOnPath({"oobe-eula-md", "installationSettings"});
+  test::OobeJS().ExpectVisiblePath(
+      {"oobe-eula-md", "installationSettingsDialog"});
+
+  test::OobeJS().CreateWaiter(
+      "$('oobe-eula-md').$$('#eula-password').textContent.trim() !== ''");
+  test::OobeJS().ExpectEQ(
+      "$('oobe-eula-md').$$('#eula-password').textContent.trim()",
+      std::string(FakeCryptohomeClient::kStubTpmPassword));
+}
+
+// Verifies statistic collection accepted flow.
+// Advaces to the next screen and verifies stats collection is enabled.
+IN_PROC_BROWSER_TEST_F(EulaTest, EnableUsageStats) {
+  ShowEulaScreen();
+
+  // Verify that toggle is enabled by default.
+  test::OobeJS().ExpectTrue("$('oobe-eula-md').$$('#usageStats').checked");
+
+  ASSERT_TRUE(StatsReportingController::IsInitialized());
+
+  // Explicitly set as false to make sure test modifies these values.
+  StatsReportingController::Get()->SetEnabled(
+      ProfileManager::GetActiveUserProfile(), false);
+  g_browser_process->local_state()->SetBoolean(
+      metrics::prefs::kMetricsReportingEnabled, false);
+
+  WaitForBlockingCall(SetCollectStatsConsentClosure(false));
+
+  // Start Listening for StatsReportingController updates.
+  base::RunLoop runloop;
+  auto subscription =
+      StatsReportingController::Get()->AddObserver(runloop.QuitClosure());
+
+  // Advance to the next screen for changes to take effect.
+  test::OobeJS().TapOnPath({"oobe-eula-md", "acceptButton"});
+
+  // Wait for StartReporting update.
+  runloop.Run();
+
+  // Verify stats collection is enabled.
+  EXPECT_TRUE(StatsReportingController::Get()->IsEnabled());
+  EXPECT_TRUE(g_browser_process->local_state()->GetBoolean(
+      metrics::prefs::kMetricsReportingEnabled));
+  EXPECT_TRUE(EvaluateBlocking(
+      base::BindOnce(&GoogleUpdateSettings::GetCollectStatsConsent)));
+}
+
+// Verify statistic collection denied flow. Clicks on usage stats toggle,
+// advaces to the next screen and verifies stats collection is disabled.
+IN_PROC_BROWSER_TEST_F(EulaTest, DisableUsageStats) {
+  ShowEulaScreen();
+
+  // Verify that toggle is enabled by default.
+  test::OobeJS().ExpectTrue("$('oobe-eula-md').$$('#usageStats').checked");
+
+  ASSERT_TRUE(StatsReportingController::IsInitialized());
+
+  // Explicitly set as true to make sure test modifies these values.
+  StatsReportingController::Get()->SetEnabled(
+      ProfileManager::GetActiveUserProfile(), true);
+  g_browser_process->local_state()->SetBoolean(
+      metrics::prefs::kMetricsReportingEnabled, true);
+
+  WaitForBlockingCall(SetCollectStatsConsentClosure(true));
+
+  // Start Listening for StatsReportingController updates.
+  base::RunLoop runloop;
+  auto subscription =
+      StatsReportingController::Get()->AddObserver(runloop.QuitClosure());
+
+  // Click on the toggle to disable stats collection and advance to the next
+  // screen for changes to take effect.
+  NonPolymerOobeJS().TapOnPath({"oobe-eula-md", "usageStats"});
+  test::OobeJS().TapOnPath({"oobe-eula-md", "acceptButton"});
+
+  // Wait for StartReportingController update.
+  runloop.Run();
+
+  // Verify stats collection is disabled.
+  EXPECT_FALSE(StatsReportingController::Get()->IsEnabled());
+  EXPECT_FALSE(g_browser_process->local_state()->GetBoolean(
+      metrics::prefs::kMetricsReportingEnabled));
+  EXPECT_FALSE(EvaluateBlocking(
+      base::BindOnce(&GoogleUpdateSettings::GetCollectStatsConsent)));
+}
+
+// Tests that clicking on "Learn more" button opens a help dialog.
+IN_PROC_BROWSER_TEST_F(EulaTest, LearnMore) {
+  ShowEulaScreen();
+
+  // Load HelperApp extension.
+  HelpAppTestHelper scoped_helper;
+
+  // Start listening for help dialog creation.
+  HelpAppTestHelper::Waiter waiter;
+
+  NonPolymerOobeJS().TapOnPath({"oobe-eula-md", "learn-more"});
+
+  // Wait until help dialog is displayed.
+  waiter.Wait();
+}
+
 }  // namespace
 }  // namespace chromeos

chrome/browser/chromeos/login/help_app_launcher.cc

@@ -24,8 +24,12 @@ using extensions::ExtensionRegistry;
 
 namespace {
 
-const char kHelpAppFormat[] =
-    "chrome-extension://honijodknafkokifofgiaalefdiedpko/oobe.html?id=%d";
+// Official HelpApp extension id.
+const char kExtensionId[] = "honijodknafkokifofgiaalefdiedpko";
+
+const char kHelpAppFormat[] = "chrome-extension://%s/oobe.html?id=%d";
+
+const char* g_extension_id_for_test = nullptr;
 
 }  // namespace
 
@@ -45,13 +49,24 @@ void HelpAppLauncher::ShowHelpTopic(HelpTopic help_topic_id) {
   if (!registry)
     return;
 
-  GURL url(base::StringPrintf(kHelpAppFormat, static_cast<int>(help_topic_id)));
+  const char* extension_id = kExtensionId;
+  if (g_extension_id_for_test && *g_extension_id_for_test != '\0') {
+    extension_id = g_extension_id_for_test;
+  }
+
+  GURL url(base::StringPrintf(kHelpAppFormat, extension_id,
+                              static_cast<int>(help_topic_id)));
   // HelpApp component extension presents only in official builds so we can
   // show help only when the extensions is installed.
   if (registry->enabled_extensions().GetByID(url.host()))
     ShowHelpTopicDialog(profile, GURL(url));
 }
 
+// static
+void HelpAppLauncher::SetExtensionIdForTest(const char* extension_id) {
+  g_extension_id_for_test = extension_id;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // HelpApp, protected:
 

chrome/browser/chromeos/login/help_app_launcher.h

@@ -47,6 +47,9 @@ class HelpAppLauncher : public base::RefCountedThreadSafe<HelpAppLauncher> {
   // Shows specified help topic.
   void ShowHelpTopic(HelpTopic help_topic_id);
 
+  // Allows tests to specify a different extension id to connect to.
+  static void SetExtensionIdForTest(const char* extension_id);
+
  protected:
   virtual ~HelpAppLauncher();
 

chrome/browser/chromeos/login/screens/eula_screen.cc

@@ -44,24 +44,6 @@ EulaScreen::~EulaScreen() {
     view_->Unbind();
 }
 
-GURL EulaScreen::GetOemEulaUrl() const {
-  const StartupCustomizationDocument* customization =
-      StartupCustomizationDocument::GetInstance();
-  if (customization->IsReady()) {
-    // Previously we're using "initial locale" that device initially
-    // booted with out-of-box. http://crbug.com/145142
-    std::string locale = g_browser_process->GetApplicationLocale();
-    std::string eula_page = customization->GetEULAPage(locale);
-    if (!eula_page.empty())
-      return GURL(eula_page);
-
-    VLOG(1) << "No eula found for locale: " << locale;
-  } else {
-    LOG(ERROR) << "No manifest found.";
-  }
-  return GURL();
-}
-
 void EulaScreen::InitiatePasswordFetch() {
   if (tpm_password_.empty()) {
     password_fetcher_.Fetch();

chrome/browser/chromeos/login/test/help_app_test_helper.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/chromeos/login/test/help_app_test_helper.h"
+
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "chrome/browser/chromeos/extensions/signin_screen_policy_provider.h"
+#include "chrome/browser/chromeos/login/help_app_launcher.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/extensions/chrome_test_extension_loader.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/grit/generated_resources.h"
+#include "ui/aura/env.h"
+#include "ui/aura/window.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/views/widget/widget.h"
+
+namespace chromeos {
+
+///////////////////////////////////////////////////////////////////////////////
+// HelpAppTestHelper::Waiter
+///////////////////////////////////////////////////////////////////////////////
+
+HelpAppTestHelper::Waiter::Waiter() {
+  aura::Env::GetInstance()->AddObserver(this);
+}
+
+HelpAppTestHelper::Waiter::~Waiter() {
+  aura::Env::GetInstance()->RemoveObserver(this);
+  window_observer_.Remove(dialog_window_);
+  // Explicitly close the dialog to prevent flaky errors in browser tests.
+  views::Widget::GetWidgetForNativeView(dialog_window_)->CloseNow();
+}
+
+void HelpAppTestHelper::Waiter::Wait() {
+  if (!dialog_window_)
+    run_loop_.Run();
+}
+
+void HelpAppTestHelper::Waiter::OnWindowInitialized(aura::Window* window) {
+  window_observer_.Add(window);
+}
+
+void HelpAppTestHelper::Waiter::OnWindowVisibilityChanged(aura::Window* window,
+                                                          bool visible) {
+  if (IsHelpAppDialog(window) && visible && !dialog_window_) {
+    dialog_window_ = window;
+    run_loop_.QuitClosure().Run();
+  }
+}
+
+bool HelpAppTestHelper::Waiter::IsHelpAppDialog(aura::Window* window) {
+  return window->GetTitle() ==
+         l10n_util::GetStringUTF16(IDS_LOGIN_OOBE_HELP_DIALOG_TITLE);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// HelpAppTestHelper
+///////////////////////////////////////////////////////////////////////////////
+
+HelpAppTestHelper::HelpAppTestHelper() {
+  auto reset = GetScopedSigninScreenPolicyProviderDisablerForTesting();
+
+  base::FilePath test_data_dir;
+  base::PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
+  extensions::ChromeTestExtensionLoader loader(
+      ProfileHelper::GetSigninProfile());
+  loader.set_allow_incognito_access(true);
+
+  scoped_refptr<const extensions::Extension> extension =
+      loader.LoadExtension(test_data_dir.AppendASCII("extensions")
+                               .AppendASCII("api_test")
+                               .AppendASCII("help_app"));
+
+  DCHECK(extension && !extension->id().empty());
+
+  HelpAppLauncher::SetExtensionIdForTest(extension->id().c_str());
+}
+
+HelpAppTestHelper::~HelpAppTestHelper() {
+  HelpAppLauncher::SetExtensionIdForTest(nullptr);
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/test/help_app_test_helper.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_CHROMEOS_LOGIN_TEST_HELP_APP_TEST_HELPER_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_HELP_APP_TEST_HELPER_H_
+
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "base/scoped_observer.h"
+#include "ui/aura/env_observer.h"
+#include "ui/aura/window_observer.h"
+
+namespace chromeos {
+
+// Provides utilies for launching and interacting with HelpApp dialogs in tests.
+class HelpAppTestHelper {
+ public:
+  // Waits for a HelpApp dialog to open and become visible.
+  class Waiter : public aura::EnvObserver, public aura::WindowObserver {
+   public:
+    Waiter();
+    ~Waiter() override;
+
+    // Blocks until a HelpApp dialog becomes visible.
+    void Wait();
+
+    // aura::EnvObserver
+    void OnWindowInitialized(aura::Window* window) override;
+
+    // aura::WindowObserver
+    void OnWindowVisibilityChanged(aura::Window* window, bool visible) override;
+
+   private:
+    bool IsHelpAppDialog(aura::Window* window);
+
+    base::RunLoop run_loop_;
+    aura::Window* dialog_window_ = nullptr;
+    ScopedObserver<aura::Window, aura::WindowObserver> window_observer_{this};
+    DISALLOW_COPY_AND_ASSIGN(Waiter);
+  };
+
+  HelpAppTestHelper();
+  virtual ~HelpAppTestHelper();
+
+  // Performs setup to allow HelpApp dialogs to open in tests.
+  void SetUpHelpAppForTest();
+
+  // TODO(tonydeluna): Add utilities for interacting with the contents of the
+  // help dialogs.
+
+  DISALLOW_COPY_AND_ASSIGN(HelpAppTestHelper);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_TEST_HELP_APP_TEST_HELPER_H_

chrome/browser/chromeos/login/test/js_checker.cc

@@ -237,11 +237,11 @@ void JSChecker::ExpectHasNoClass(
 void JSChecker::TapOnPath(
     std::initializer_list<base::StringPiece> element_ids) {
   ExpectVisiblePath(element_ids);
-  // All OOBE UI should be mobile-friendly, so use "tap" instead of "click".
+  // TODO(crbug.com/949377): Switch to always firing 'click' events when
+  // missing OOBE UI components are migrated to handle 'click' events.
   if (polymer_ui_) {
     Evaluate(GetOobeElementPath(element_ids) + ".fire('tap')");
   } else {
-    // Old test-only UI (fake GAIA, fake SAML) only support "click".
     Evaluate(GetOobeElementPath(element_ids) + ".click()");
   }
 }

chrome/browser/resources/chromeos/login/oobe_eula.html

@@ -71,7 +71,7 @@
           </cr-toggle>
           <div id="usageStatsLabelContainer">
             <span id="usageStatsLabel" i18n-content="checkboxLogging"></span>
-            <a id="" href="#" i18n-content="learnMore"
+            <a id="learn-more" href="#" i18n-content="learnMore"
                 on-tap="onUsageStatsHelpLinkClicked_">
             </a>
           </div>
@@ -109,7 +109,7 @@
           </div>
           <div class='password-row layout horizontal'>
             <div class="flex"></div>
-            <div>{{password}}</div>
+            <div id="eula-password">{{password}}</div>
             <div class="flex"></div>
           </div>
         </div>

chrome/test/data/extensions/api_test/help_app/manifest.json

+{
+  "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuCAA3/rQ/X5xUBc2DhjXGB7U0Fn6memyWN5u3XYc1QzNdKodI5hmqVRi6yVePzmQuSdsA2M7wG0enGkg7qfQKNydcHPM7z/J0GEmSqzadrtz5QzWN2JemkIB3ilB7gO1RcyEluuHDhepCbVCwIr3VbguaaoatW4zYndqgC9mNOF5M73G44s6GkCunNcHtUaleaRiwt12y+WZsKIfRW90ijyESlSOEmGVhWwMld9SnAJv/P7apNr2HfQR4Bemriu02ypAPuJQ6zpvK+DRFdXu1bb7gOuN10+zPCjrt8SLs79rZ974bA56gAqUjDf3kLtJiHXMavCPUf3Fs1mvpzKTJwIDAQAB",
+  "name": "Help",
+  "version": "4.0",
+  "description" : "Test Chrome OS Help",
+  "manifest_version": 2,
+  "incognito" : "split"
+}

chrome/test/data/extensions/api_test/help_app/oobe.html

+<!DOCTYPE html>
+<html>
+<head>
+<title>Fake Help App</title>
+</head>
+<body>
+  In official builds this shows help pages.
+</body>
+</html>

chromeos/dbus/cryptohome/fake_cryptohome_client.cc

@@ -47,6 +47,9 @@ FakeCryptohomeClient* g_instance = nullptr;
 
 }  // namespace
 
+// static
+constexpr char FakeCryptohomeClient::kStubTpmPassword[] = "Stub-TPM-password";
+
 FakeCryptohomeClient::FakeCryptohomeClient()
     : service_is_available_(true),
       remove_firmware_management_parameters_from_tpm_call_count_(0),
@@ -184,7 +187,6 @@ bool FakeCryptohomeClient::CallTpmIsEnabledAndBlock(bool* enabled) {
 
 void FakeCryptohomeClient::TpmGetPassword(
     DBusMethodCallback<std::string> callback) {
-  constexpr char kStubTpmPassword[] = "Stub-TPM-password";
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(std::move(callback), std::string(kStubTpmPassword)));

chromeos/dbus/cryptohome/fake_cryptohome_client.h

@@ -36,6 +36,9 @@ class COMPONENT_EXPORT(CRYPTOHOME_CLIENT) FakeCryptohomeClient
   // Checks that a FakeCryptohome instance was initialized and returns it.
   static FakeCryptohomeClient* Get();
 
+  // Expose stub password for tests.
+  static const char kStubTpmPassword[];
+
   // CryptohomeClient overrides
   void AddObserver(Observer* observer) override;
   void RemoveObserver(Observer* observer) override;