Improve UpdateScreen browser tests

Updates UpdateScreen browser tests to test UI state changes in addition
to interactions with Chrome OS system state.

Adds coverage for testing the screen state when update requires user
confirmation to proceed (when the current network connection is
detected as metered), and reboot request timeout.

To make testing easier:
*   extracts logic for setting up network portal
    state into NetworkPortalDetectorMixin (this will be reused for
    other tests).
*   refactors OobeScreen waiter to have only one Wait method, and wait
    parameters are expected to be set before a call to Wait()
    *   introduces an option to confirm that no other oobe screen is
        shown before the target screen
    *   remove unused WaitForInitialization

BUG=945133

Change-Id: I1c9899f9adad6d70a5de5d206fa1d3adf2f7e06f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1536112Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644663}

chrome/browser/chromeos/login/kiosk_browsertest.cc

@@ -993,8 +993,11 @@ IN_PROC_BROWSER_TEST_F(KioskTest, DISABLED_LaunchAppNetworkPortal) {
 
   // Start app launch with network portal state.
   StartAppLaunchFromLoginScreen(SimulateNetworkPortalClosure());
-  OobeScreenWaiter(OobeScreen::SCREEN_APP_LAUNCH_SPLASH)
-      .WaitNoAssertCurrentScreen();
+
+  OobeScreenWaiter app_splash_waiter(OobeScreen::SCREEN_APP_LAUNCH_SPLASH);
+  app_splash_waiter.set_no_assert_last_screen();
+  app_splash_waiter.Wait();
+
   WaitForAppLaunchNetworkTimeout();
 
   // Network error should show up automatically since this test does not

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

@@ -15,6 +15,7 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/time/default_tick_clock.h"
 #include "chrome/browser/chromeos/login/error_screens_histogram_helper.h"
 #include "chrome/browser/chromeos/login/screen_manager.h"
 #include "chrome/browser/chromeos/login/screens/base_screen_delegate.h"
@@ -95,6 +96,7 @@ UpdateScreen::UpdateScreen(BaseScreenDelegate* base_screen_delegate,
                            UpdateView* view,
                            const ScreenExitCallback& exit_callback)
     : BaseScreen(OobeScreen::SCREEN_OOBE_UPDATE),
+      tick_clock_(base::DefaultTickClock::GetInstance()),
       reboot_check_delay_(kWaitForRebootTimeSec),
       base_screen_delegate_(base_screen_delegate),
       view_(view),
@@ -186,7 +188,7 @@ void UpdateScreen::UpdateStatusChanged(
         // Because update engine doesn't send UPDATE_STATUS_UPDATE_AVAILABLE
         // we need to is update critical on first downloading notification.
         is_downloading_update_ = true;
-        download_start_time_ = download_last_time_ = base::Time::Now();
+        download_start_time_ = download_last_time_ = tick_clock_->NowTicks();
         download_start_progress_ = status.download_progress;
         download_last_progress_ = status.download_progress;
         is_download_average_speed_computed_ = false;
@@ -334,9 +336,12 @@ void UpdateScreen::CancelUpdate() {
   ExitUpdate(Result::UPDATE_NOT_REQUIRED);
 }
 
-// TODO(jdufault): This should return a pointer. See crbug.com/672142.
-base::OneShotTimer& UpdateScreen::GetErrorMessageTimerForTesting() {
-  return error_message_timer_;
+base::OneShotTimer* UpdateScreen::GetErrorMessageTimerForTesting() {
+  return &error_message_timer_;
+}
+
+base::OneShotTimer* UpdateScreen::GetRebootTimerForTesting() {
+  return &reboot_timer_;
 }
 
 void UpdateScreen::Show() {
@@ -406,7 +411,7 @@ void UpdateScreen::RetryUpdateWithUpdateOverCellularPermissionSet(
 
 void UpdateScreen::UpdateDownloadingStats(
     const UpdateEngineClient::Status& status) {
-  base::Time download_current_time = base::Time::Now();
+  base::TimeTicks download_current_time = tick_clock_->NowTicks();
   if (download_current_time >=
       download_last_time_ +
           base::TimeDelta::FromSeconds(kMinTimeStepInSeconds)) {
@@ -456,7 +461,7 @@ void UpdateScreen::UpdateDownloadingStats(
 }
 
 bool UpdateScreen::HasCriticalUpdate() {
-  if (is_ignore_update_deadlines_)
+  if (ignore_update_deadlines_)
     return true;
 
   std::string deadline;

chrome/browser/chromeos/login/screens/update_screen.h

@@ -17,6 +17,10 @@
 #include "chromeos/dbus/update_engine_client.h"
 #include "chromeos/network/portal_detector/network_portal_detector.h"
 
+namespace base {
+class TickClock;
+}
+
 namespace chromeos {
 
 class BaseScreenDelegate;
@@ -89,14 +93,20 @@ class UpdateScreen : public BaseScreen,
   // Skip update UI, usually used only in debug builds/tests.
   void CancelUpdate();
 
-  base::OneShotTimer& GetErrorMessageTimerForTesting();
+  // BaseScreen:
+  void Show() override;
+  void Hide() override;
+  void OnUserAction(const std::string& action_id) override;
+
+  base::OneShotTimer* GetErrorMessageTimerForTesting();
+  base::OneShotTimer* GetRebootTimerForTesting();
 
-  void set_base_screen_delegate_for_testing(BaseScreenDelegate* delegate) {
-    base_screen_delegate_ = delegate;
+  void set_ignore_update_deadlines_for_testing(bool ignore_update_deadlines) {
+    ignore_update_deadlines_ = ignore_update_deadlines;
   }
 
-  void set_exit_callback_for_testing(const ScreenExitCallback& callback) {
-    exit_callback_ = callback;
+  void set_tick_clock_for_testing(const base::TickClock* tick_clock) {
+    tick_clock_ = tick_clock;
   }
 
  protected:
@@ -117,11 +127,6 @@ class UpdateScreen : public BaseScreen,
     STATE_ERROR
   };
 
-  // BaseScreen:
-  void Show() override;
-  void Hide() override;
-  void OnUserAction(const std::string& action_id) override;
-
   // Callback to UpdateEngineClient::SetUpdateOverCellularOneTimePermission
   // called in response to user confirming that the OS update can proceed
   // despite being over cellular charges.
@@ -168,6 +173,8 @@ class UpdateScreen : public BaseScreen,
   // Current state of the update screen.
   State state_ = State::STATE_IDLE;
 
+  const base::TickClock* tick_clock_;
+
   // Time in seconds after which we decide that the device has not rebooted
   // automatically. If reboot didn't happen during this interval, ask user to
   // reboot device manually.
@@ -179,7 +186,7 @@ class UpdateScreen : public BaseScreen,
   bool is_downloading_update_ = false;
   // If true, update deadlines are ignored.
   // Note, this is false by default.
-  bool is_ignore_update_deadlines_ = false;
+  bool ignore_update_deadlines_ = false;
   // Whether the update screen is shown.
   bool is_shown_ = false;
   // Ignore fist IDLE status that is sent before update screen initiated check.
@@ -190,11 +197,11 @@ class UpdateScreen : public BaseScreen,
   ScreenExitCallback exit_callback_;
 
   // Time of the first notification from the downloading stage.
-  base::Time download_start_time_;
+  base::TimeTicks download_start_time_;
   double download_start_progress_ = 0;
 
   // Time of the last notification from the downloading stage.
-  base::Time download_last_time_;
+  base::TimeTicks download_last_time_;
   double download_last_progress_ = 0;
 
   bool is_download_average_speed_computed_ = false;

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

@@ -52,7 +52,7 @@ class UpdateScreenUnitTest : public testing::Test {
     if (critical) {
       ASSERT_TRUE(available) << "Does not make sense for an update to be "
                                 "critical if one is not even available.";
-      update_screen->is_ignore_update_deadlines_ = true;
+      update_screen->set_ignore_update_deadlines_for_testing(true);
     }
     update_engine_status.status =
         available ? UpdateEngineClient::UPDATE_STATUS_UPDATE_AVAILABLE

chrome/browser/chromeos/login/test/network_portal_detector_mixin.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/network_portal_detector_mixin.h"
+
+#include "base/run_loop.h"
+#include "chrome/browser/chromeos/net/network_portal_detector_test_impl.h"
+#include "chromeos/dbus/fake_shill_manager_client.h"
+
+namespace chromeos {
+
+NetworkPortalDetectorMixin::NetworkPortalDetectorMixin(
+    InProcessBrowserTestMixinHost* host)
+    : InProcessBrowserTestMixin(host) {}
+
+NetworkPortalDetectorMixin::~NetworkPortalDetectorMixin() = default;
+
+void NetworkPortalDetectorMixin::SetDefaultNetwork(
+    const std::string& network_guid,
+    NetworkPortalDetector::CaptivePortalStatus status) {
+  network_portal_detector_->SetDefaultNetworkForTesting(network_guid);
+
+  SimulateDefaultNetworkState(status);
+}
+
+void NetworkPortalDetectorMixin::SimulateNoNetwork() {
+  network_portal_detector_->SetDefaultNetworkForTesting("");
+
+  network_portal_detector_->NotifyObserversForTesting();
+}
+
+void NetworkPortalDetectorMixin::SimulateDefaultNetworkState(
+    NetworkPortalDetector::CaptivePortalStatus status) {
+  std::string default_network_guid =
+      network_portal_detector_->GetDefaultNetworkGuid();
+  DCHECK(!default_network_guid.empty());
+
+  NetworkPortalDetector::CaptivePortalState detection_state;
+  detection_state.status = status;
+  if (status == NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE) {
+    detection_state.response_code = 204;
+  } else if (status == NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL) {
+    detection_state.response_code = 200;
+  } else {
+    detection_state.response_code = -1;
+  }
+
+  network_portal_detector_->SetDetectionResultsForTesting(default_network_guid,
+                                                          detection_state);
+  network_portal_detector_->NotifyObserversForTesting();
+}
+
+void NetworkPortalDetectorMixin::WaitForPortalDetectionRequest() {
+  if (network_portal_detector_->portal_detection_in_progress())
+    return;
+  base::RunLoop run_loop;
+  network_portal_detector_->RegisterPortalDetectionStartCallback(
+      run_loop.QuitClosure());
+  run_loop.Run();
+}
+
+void NetworkPortalDetectorMixin::SetUpInProcessBrowserTestFixture() {
+  // Setup network portal detector to return online state for both
+  // ethernet and wifi networks. Ethernet is an active network by
+  // default.
+  network_portal_detector_ = new NetworkPortalDetectorTestImpl();
+  network_portal_detector::InitializeForTesting(network_portal_detector_);
+  SetDefaultNetwork(FakeShillManagerClient::kFakeEthernetNetworkGuid,
+                    NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE);
+}
+void NetworkPortalDetectorMixin::TearDownInProcessBrowserTestFixture() {
+  network_portal_detector::Shutdown();
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/test/network_portal_detector_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_CHROMEOS_LOGIN_TEST_NETWORK_PORTAL_DETECTOR_MIXIN_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_NETWORK_PORTAL_DETECTOR_MIXIN_H_
+
+#include "chrome/browser/chromeos/login/mixin_based_in_process_browser_test.h"
+#include "chromeos/network/portal_detector/network_portal_detector.h"
+
+namespace chromeos {
+
+class NetworkPortalDetectorTestImpl;
+
+// An InBrowserProcessTestMixin that provides utility methods for faking
+// network captive portal detector state.
+class NetworkPortalDetectorMixin : public InProcessBrowserTestMixin {
+ public:
+  explicit NetworkPortalDetectorMixin(InProcessBrowserTestMixinHost* host);
+  ~NetworkPortalDetectorMixin() override;
+
+  NetworkPortalDetectorMixin(const NetworkPortalDetectorMixin& other) = delete;
+  NetworkPortalDetectorMixin& operator=(
+      const NetworkPortalDetectorMixin& other) = delete;
+
+  // Changes the default network as seen by network portal detector, sets its
+  // state and notifies NetworkPortalDetector observers of the portal detection
+  // completion.
+  void SetDefaultNetwork(const std::string& network_guid,
+                         NetworkPortalDetector::CaptivePortalStatus status);
+
+  // Simulates no network state. It notifies NetworkPortalDetector observers of
+  // the portal detection state.
+  void SimulateNoNetwork();
+
+  // Sets the default network's captive portal state. It notifies
+  // NetworkPortalDetector observers of the new portal detection state.
+  void SimulateDefaultNetworkState(
+      NetworkPortalDetector::CaptivePortalStatus status);
+
+  // Runs loop until the NetworkPortalDetector is requested to start portal
+  // detection. It will return immediately if a detection is already in
+  // progress.
+  void WaitForPortalDetectionRequest();
+
+  // InProcessBrowserTestMixin:
+  void SetUpInProcessBrowserTestFixture() override;
+  void TearDownInProcessBrowserTestFixture() override;
+
+ private:
+  NetworkPortalDetectorTestImpl* network_portal_detector_ = nullptr;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_TEST_NETWORK_PORTAL_DETECTOR_MIXIN_H_

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

@@ -4,73 +4,82 @@
 
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
 
+#include "base/run_loop.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
-#include "content/public/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 
-OobeScreenWaiter::OobeScreenWaiter(OobeScreen expected_screen)
-    : expected_screen_(expected_screen) {}
+OobeScreenWaiter::OobeScreenWaiter(OobeScreen target_screen)
+    : target_screen_(target_screen) {}
 
-OobeScreenWaiter::~OobeScreenWaiter() {
-  if (waiting_for_screen_ || waiting_for_screen_init_) {
-    GetOobeUI()->RemoveObserver(this);
-  }
-}
+OobeScreenWaiter::~OobeScreenWaiter() = default;
 
 void OobeScreenWaiter::Wait() {
-  WaitNoAssertCurrentScreen();
+  DCHECK_EQ(State::IDLE, state_);
 
-  ASSERT_EQ(expected_screen_, GetOobeUI()->current_screen());
-}
-
-void OobeScreenWaiter::WaitForInitialization() {
-  if (GetOobeUI()->IsScreenInitialized(expected_screen_))
+  if (GetOobeUI()->current_screen() == target_screen_) {
+    state_ = State::DONE;
     return;
+  }
+  DCHECK(!run_loop_);
 
-  waiting_for_screen_init_ = true;
-  GetOobeUI()->AddObserver(this);
+  oobe_ui_observer_.Add(GetOobeUI());
 
-  runner_ = new content::MessageLoopRunner;
-  runner_->Run();
-  ASSERT_FALSE(waiting_for_screen_init_);
-  ASSERT_TRUE(GetOobeUI()->IsScreenInitialized(expected_screen_));
-}
+  state_ = State::WAITING_FOR_SCREEN;
 
-void OobeScreenWaiter::WaitNoAssertCurrentScreen() {
-  if (GetOobeUI()->current_screen() == expected_screen_)
-    return;
+  run_loop_ = std::make_unique<base::RunLoop>();
+  run_loop_->Run();
+  run_loop_.reset();
 
-  waiting_for_screen_ = true;
-  GetOobeUI()->AddObserver(this);
+  DCHECK_EQ(State::DONE, state_);
 
-  runner_ = new content::MessageLoopRunner;
-  runner_->Run();
-  ASSERT_FALSE(waiting_for_screen_);
+  oobe_ui_observer_.RemoveAll();
+
+  if (assert_last_screen_)
+    EXPECT_EQ(target_screen_, GetOobeUI()->current_screen());
 }
 
 void OobeScreenWaiter::OnCurrentScreenChanged(OobeScreen current_screen,
                                               OobeScreen new_screen) {
-  if (waiting_for_screen_ && new_screen == expected_screen_) {
-    runner_->Quit();
-    waiting_for_screen_ = false;
-    GetOobeUI()->RemoveObserver(this);
+  DCHECK_NE(state_, State::IDLE);
+
+  if (state_ != State::WAITING_FOR_SCREEN) {
+    if (assert_last_screen_ && new_screen != target_screen_) {
+      ADD_FAILURE() << "Screen changed from the target screen "
+                    << static_cast<int>(current_screen) << " -> "
+                    << static_cast<int>(new_screen);
+      EndWait();
+    }
+    return;
   }
-}
 
-void OobeScreenWaiter::OnScreenInitialized(OobeScreen screen) {
-  if (waiting_for_screen_init_ && screen == expected_screen_) {
-    runner_->Quit();
-    waiting_for_screen_init_ = false;
-    GetOobeUI()->RemoveObserver(this);
+  if (assert_next_screen_ && new_screen != target_screen_) {
+    ADD_FAILURE() << "Untarget screen change to "
+                  << static_cast<int>(new_screen) << " while waiting for "
+                  << static_cast<int>(target_screen_);
+    EndWait();
+    return;
   }
+
+  if (new_screen == target_screen_)
+    EndWait();
 }
 
+void OobeScreenWaiter::OnScreenInitialized(OobeScreen screen) {}
+
 OobeUI* OobeScreenWaiter::GetOobeUI() {
   OobeUI* oobe_ui = LoginDisplayHost::default_host()->GetOobeUI();
   CHECK(oobe_ui);
   return oobe_ui;
 }
 
+void OobeScreenWaiter::EndWait() {
+  if (state_ == State::DONE)
+    return;
+
+  state_ = State::DONE;
+  run_loop_->Quit();
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/login/test/oobe_screen_waiter.h

@@ -6,31 +6,28 @@
 #define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_OOBE_SCREEN_WAITER_H_
 
 #include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "chrome/browser/chromeos/login/oobe_screen.h"
 #include "chrome/browser/chromeos/login/test/test_condition_waiter.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 
-namespace content {
-class MessageLoopRunner;
+namespace base {
+class RunLoop;
 }
 
 namespace chromeos {
 
-// A waiter that blocks until the expected oobe screen is reached.
+class OobeUI;
+
+// A waiter that blocks until the target oobe screen is reached.
 class OobeScreenWaiter : public OobeUI::Observer,
                          public test::TestConditionWaiter {
  public:
-  explicit OobeScreenWaiter(OobeScreen expected_screen);
+  explicit OobeScreenWaiter(OobeScreen target_screen);
   ~OobeScreenWaiter() override;
 
-
-  // Run message loop to wait for the expected_screen to be fully initialized.
-  void WaitForInitialization();
-
-  // Similar to Wait() but does not assert the current screen is
-  // expected_screen on exit. Use this when there are multiple screen changes
-  // during the wait and the screen to be waited is not the final one.
-  void WaitNoAssertCurrentScreen();
+  void set_no_assert_last_screen() { assert_last_screen_ = false; }
+  void set_assert_next_screen() { assert_next_screen_ = true; }
 
   // OobeUI::Observer implementation:
   void OnCurrentScreenChanged(OobeScreen current_screen,
@@ -41,12 +38,27 @@ class OobeScreenWaiter : public OobeUI::Observer,
   void Wait() override;
 
  private:
+  enum class State { IDLE, WAITING_FOR_SCREEN, DONE };
+
   OobeUI* GetOobeUI();
+  void EndWait();
+
+  const OobeScreen target_screen_;
+
+  State state_ = State::IDLE;
+
+  // If set, waiter will assert that the OOBE UI does not transition to any
+  // other screen after the transition to the target screen.
+  // This applies to the time period Wait() is running only.
+  bool assert_last_screen_ = true;
+
+  // If set, the watier will assert OOBE UI does not transition to any other
+  // screen before transisioning to the target screen.
+  bool assert_next_screen_ = false;
+
+  ScopedObserver<OobeUI, OobeScreenWaiter> oobe_ui_observer_{this};
 
-  bool waiting_for_screen_init_ = false;
-  bool waiting_for_screen_ = false;
-  OobeScreen expected_screen_ = OobeScreen::SCREEN_UNKNOWN;
-  scoped_refptr<content::MessageLoopRunner> runner_;
+  std::unique_ptr<base::RunLoop> run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(OobeScreenWaiter);
 };

chrome/browser/chromeos/net/network_portal_detector_test_impl.cc

@@ -41,10 +41,23 @@ void NetworkPortalDetectorTestImpl::NotifyObserversForTesting() {
   CaptivePortalState state;
   if (default_network_ && portal_state_map_.count(default_network_->guid()))
     state = portal_state_map_[default_network_->guid()];
+  portal_detection_in_progress_ = false;
   for (auto& observer : observers_)
     observer.OnPortalDetectionCompleted(default_network_.get(), state);
 }
 
+std::string NetworkPortalDetectorTestImpl::GetDefaultNetworkGuid() const {
+  if (!default_network_)
+    return "";
+
+  return default_network_->guid();
+}
+
+void NetworkPortalDetectorTestImpl::RegisterPortalDetectionStartCallback(
+    base::OnceClosure callback) {
+  start_detection_callbacks_.push_back(std::move(callback));
+}
+
 void NetworkPortalDetectorTestImpl::AddObserver(Observer* observer) {
   if (observer && !observers_.HasObserver(observer))
     observers_.AddObserver(observer);
@@ -89,7 +102,16 @@ void NetworkPortalDetectorTestImpl::Enable(bool start_detection) {
 }
 
 bool NetworkPortalDetectorTestImpl::StartPortalDetection(bool force) {
-  return false;
+  if (portal_detection_in_progress_ && !force)
+    return false;
+
+  portal_detection_in_progress_ = true;
+  std::vector<base::OnceClosure> callbacks =
+      std::move(start_detection_callbacks_);
+  for (auto& callback : callbacks)
+    std::move(callback).Run();
+
+  return true;
 }
 
 void NetworkPortalDetectorTestImpl::SetStrategy(

chrome/browser/chromeos/net/network_portal_detector_test_impl.h

@@ -8,6 +8,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -26,6 +27,13 @@ class NetworkPortalDetectorTestImpl : public NetworkPortalDetector {
                                      const CaptivePortalState& state);
   void NotifyObserversForTesting();
 
+  // Returns the GUID of the network the detector considers to be default.
+  std::string GetDefaultNetworkGuid() const;
+
+  // Registers a callback that will be run when portal detection is requested by
+  // StartPortalDetection().
+  void RegisterPortalDetectionStartCallback(base::OnceClosure callback);
+
   // NetworkPortalDetector implementation:
   void AddObserver(Observer* observer) override;
   void AddAndFireObserver(Observer* observer) override;
@@ -41,6 +49,10 @@ class NetworkPortalDetectorTestImpl : public NetworkPortalDetector {
     return strategy_id_;
   }
 
+  bool portal_detection_in_progress() const {
+    return portal_detection_in_progress_;
+  }
+
  private:
   using NetworkId = std::string;
   using CaptivePortalStateMap = std::map<NetworkId, CaptivePortalState>;
@@ -50,6 +62,12 @@ class NetworkPortalDetectorTestImpl : public NetworkPortalDetector {
   CaptivePortalStateMap portal_state_map_;
   PortalDetectorStrategy::StrategyId strategy_id_;
 
+  // Set when StartPortalDetection() is called - it will be reset when observers
+  // are notified using NotifyObserversForTesting().
+  bool portal_detection_in_progress_ = false;
+
+  std::vector<base::OnceClosure> start_detection_callbacks_;
+
   DISALLOW_COPY_AND_ASSIGN(NetworkPortalDetectorTestImpl);
 };
 

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

@@ -27,7 +27,7 @@
     <link rel="stylesheet" href="oobe_flex_layout.css">
     <link rel="stylesheet" href="oobe_update.css">
     <oobe-dialog hidden="[[!checkingForUpdate]]" tabindex="0"
-        aria-live="polite">
+        aria-live="polite" id="checking-for-updates-dialog">
       <iron-icon icon="oobe-update:googleg" slot="oobe-icon"></iron-icon>
       <h1 slot="title" i18n-content="checkingForUpdates"></h1>
       <div slot="subtitle" class="update-subtitle" id="checkingForUpdateCancelHint"
@@ -43,7 +43,7 @@
       </div>
     </oobe-dialog>
     <oobe-dialog hidden="[[!requiresPermissionForCellular]]" tabindex="0"
-        aria-live="polite" has-buttons>
+        aria-live="polite" has-buttons id="cellular-permission-dialog">
       <iron-icon icon="oobe-update:googleg" slot="oobe-icon"></iron-icon>
       <h1 slot="title">
           [[i18nRecursive(locale, 'updateOverCellularPromptTitle',
@@ -58,12 +58,15 @@
                     images/2x/updating_2x.svg 2x">
       </div>
       <div slot="bottom-buttons" class="layout horizontal justified">
-        <oobe-back-button inverse on-tap="onBackClicked_"></oobe-back-button>
-        <oobe-next-button inverse on-tap="onNextClicked_"></oobe-next-button>
+        <oobe-back-button id="cellular-permission-back" inverse
+            on-tap="onBackClicked_"></oobe-back-button>
+        <oobe-next-button id="cellular-permission-next" inverse
+            on-tap="onNextClicked_"></oobe-next-button>
       </div>
     </oobe-dialog>
-    <oobe-dialog hidden="[[hideUpdatingScreen_(checkingForUpdate,
-                                               requiresPermissionForCellular)]]"
+    <oobe-dialog id="updating-dialog"
+        hidden="[[hideUpdatingScreen_(checkingForUpdate,
+                                      requiresPermissionForCellular)]]"
         tabindex="0" aria-live="polite">
       <iron-icon icon="oobe-update:googleg" slot="oobe-icon"></iron-icon>
       <h1 slot="title" i18n-content="updatingScreenTitle"></h1>
@@ -75,9 +78,10 @@
           hidden="[[isNotAllowedOrUpdateCompleted_(cancelAllowed,
                                                    updateCompleted)]]">
       </div>
-      <div slot="subtitle" class="update-subtitle" i18n-content="updateCompeletedMsg"
-          hidden="[[!updateCompleted]]"></div>
-      <paper-progress slot="progress" min="0" max="100"
+      <div id="update-complete-msg" slot="subtitle" class="update-subtitle"
+          i18n-content="updateCompeletedMsg" hidden="[[!updateCompleted]]">
+      </div>
+      <paper-progress id="updating-progress" slot="progress" min="0" max="100"
           value="[[progressValue]]" hidden="[[updateCompleted]]">
       </paper-progress>
       <div slot="footer" class="flex layout vertical">

chrome/test/BUILD.gn

@@ -1896,6 +1896,8 @@ test("browser_tests") {
         "../browser/chromeos/login/test/local_policy_test_server_mixin.h",
         "../browser/chromeos/login/test/login_screen_tester.cc",
         "../browser/chromeos/login/test/login_screen_tester.h",
+        "../browser/chromeos/login/test/network_portal_detector_mixin.cc",
+        "../browser/chromeos/login/test/network_portal_detector_mixin.h",
         "../browser/chromeos/login/test/oobe_base_test.cc",
         "../browser/chromeos/login/test/oobe_base_test.h",
         "../browser/chromeos/login/ui/captive_portal_window_browsertest.cc",