Revert "Fix race conditions in OobeConfiguration tests"

This reverts commit fc684f1c.

Reason for revert: TestDemoModeAcceptEula failures on ChromeOs

Original change's description:
> Fix race conditions in OobeConfiguration tests
> 
> Explicitly shut down LoginDisplayHost in OOBEConfigurationTests
> It should remove flakiness upon shutdown.
> 
> Also Fix race condition in setting up demo mode (reason for previous revert).
> 
> Reland of https://chromium-review.googlesource.com/c/chromium/src/+/1934342
> 
> Bug: 997685
> Change-Id: I0cb8f084484395a13bc3b29e8f1b7150cada8403
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1942339
> Commit-Queue: Denis Kuznetsov [CET] <antrim@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#720207}

TBR=antrim@chromium.org,rsorokin@chromium.org

Change-Id: I3834c984a692e0e6774832a1b98425809458ce2b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 997685, 1029531
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1943536Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720268}

chrome/browser/chromeos/login/configuration_based_oobe_browsertest.cc

@@ -17,7 +17,6 @@
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
 #include "chrome/browser/chromeos/login/test/oobe_configuration_waiter.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/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/ui/webui/chromeos/login/demo_preferences_screen_handler.h"
@@ -37,46 +36,42 @@
 #include "components/prefs/pref_service.h"
 #include "ui/base/ime/chromeos/input_method_util.h"
 
-namespace chromeos {
-
-namespace {
-
-// Helper class to configure resources for offline demo mode
-// right when DemoModeController is created.
-class DemoConfigRunner : public OobeUI::Observer {
- public:
-  explicit DemoConfigRunner(base::OnceClosure env_setup_callback)
-      : callback_(std::move(env_setup_callback)) {
-    OobeUI* oobe_ui = LoginDisplayHost::default_host()->GetOobeUI();
-    if (!oobe_ui) {
-      ADD_FAILURE() << "No oobe UI at time of DemoConfigRunner creation";
-    }
-    oobe_ui_observer_.Add(oobe_ui);
-  }
-
-  DemoConfigRunner(const DemoConfigRunner&) = delete;
-  DemoConfigRunner& operator=(const DemoConfigRunner&) = delete;
-
-  // OobeUI::Observer
-  void OnCurrentScreenChanged(OobeScreenId current_screen,
-                              OobeScreenId new_screen) override {
-    if (new_screen != DemoPreferencesScreenView::kScreenId)
-      return;
-    std::move(callback_).Run();
-    oobe_ui_observer_.RemoveAll();
-  }
-
-  void OnDestroyingOobeUI() override {
-    oobe_ui_observer_.RemoveAll();
-    FAIL() << "Should have reached DemoPreferences screen";
-  }
+// TODO(crbug.com/997685): Flaky on linux.
+#if defined(OS_LINUX)
+#define MAYBE_TestSelectConnectedNetwork DISABLED_TestSelectConnectedNetwork
+#define MAYBE_TestSelectNetwork DISABLED_TestSelectNetwork
+#define MAYBE_TestDeviceRequisition DISABLED_TestDeviceRequisition
+#else
+#define MAYBE_TestSelectConnectedNetwork TestSelectConnectedNetwork
+#define MAYBE_TestSelectNetwork TestSelectNetwork
+#define MAYBE_TestDeviceRequisition TestDeviceRequisition
+#endif
+
+// Disabled due to flakiness: https://crbug.com/997685.
+#define MAYBE_TestDemoModeAcceptEula DISABLED_TestDemoModeAcceptEula
+#define MAYBE_TestDemoModeOfflineNetwork DISABLED_TestDemoModeOfflineNetwork
+// Disabled on debug build due to flakiness: https://crbug.com/997685.
+#if !defined(NDEBUG)
+#define MAYBE_TestAcceptEula DISABLED_TestAcceptEula
+#define MAYBE_TestDemoModeAcceptArcTos DISABLED_TestDemoModeAcceptArcTos
+#define MAYBE_TestDemoModePreferences DISABLED_TestDemoModePreferences
+#define MAYBE_TestEnableDemoMode DISABLED_TestEnableDemoMode
+#define MAYBE_TestLeaveWelcomeScreen DISABLED_TestLeaveWelcomeScreen
+#define MAYBE_TestSwitchLanguageIME DISABLED_TestSwitchLanguageIME
+#define MAYBE_TestLeaveWelcomeScreen DISABLED_TestLeaveWelcomeScreen
+#define MAYBE_TestSkipHIDDetection DISABLED_TestSkipHIDDetection
+#else
+#define MAYBE_TestAcceptEula TestAcceptEula
+#define MAYBE_TestDemoModeAcceptArcTos TestDemoModeAcceptArcTos
+#define MAYBE_TestDemoModePreferences TestDemoModePreferences
+#define MAYBE_TestEnableDemoMode TestEnableDemoMode
+#define MAYBE_TestLeaveWelcomeScreen TestLeaveWelcomeScreen
+#define MAYBE_TestSwitchLanguageIME TestSwitchLanguageIME
+#define MAYBE_TestLeaveWelcomeScreen TestLeaveWelcomeScreen
+#define MAYBE_TestSkipHIDDetection TestSkipHIDDetection
+#endif
 
- private:
-  ScopedObserver<OobeUI, OobeUI::Observer> oobe_ui_observer_{this};
-  base::OnceClosure callback_;
-};
-
-}  // namespace
+namespace chromeos {
 
 // This test case will use
 // src/chromeos/test/data/oobe_configuration/<TestName>.json file as
@@ -100,15 +95,12 @@ class OobeConfigurationTest : public OobeBaseTest {
     const bool ready = waiter.IsConfigurationLoaded(run_loop.QuitClosure());
     if (!ready)
       run_loop.Run();
-  }
 
-  void SimulateOfflineEnvironment() {
-    demo_config_runner_ = std::make_unique<DemoConfigRunner>(
-        base::BindOnce(&OobeConfigurationTest::SetupOfflineEnvironment,
-                       base::Unretained(this)));
+    // Let screens to settle.
+    base::RunLoop().RunUntilIdle();
   }
 
-  void SetupOfflineEnvironment() {
+  void SimulateOfflineEnvironment() {
     DemoSetupController* controller =
         WizardController::default_controller()->demo_setup_controller();
 
@@ -174,20 +166,9 @@ class OobeConfigurationTest : public OobeBaseTest {
     NetworkHandler::Get()->network_state_handler()->SetCheckPortalList("");
   }
 
-  void TearDownOnMainThread() override {
-    base::RunLoop run_loop;
-    // Explicitly shut down all login UI before shutting down browser to avoid
-    // a case when OOBE configuration object is destroyed first.
-    LoginDisplayHost::default_host()->Finalize(run_loop.QuitClosure());
-    run_loop.Run();
-    OobeBaseTest::TearDownOnMainThread();
-  }
-
  protected:
   // Owned by DBusThreadManagerSetter
   chromeos::FakeUpdateEngineClient* fake_update_engine_client_;
-
-  std::unique_ptr<DemoConfigRunner> demo_config_runner_;
   std::unique_ptr<base::AutoReset<bool>> branded_build_override_;
   base::ScopedTempDir fake_policy_dir_;
 
@@ -230,13 +211,13 @@ class OobeConfigurationEnrollmentTest : public OobeConfigurationTest {
 };
 
 // Check that configuration lets correctly pass Welcome screen.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestLeaveWelcomeScreen) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestLeaveWelcomeScreen) {
   LoadConfiguration();
   OobeScreenWaiter(NetworkScreenView::kScreenId).Wait();
 }
 
 // Check that language and input methods are set correctly.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestSwitchLanguageIME) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestSwitchLanguageIME) {
   LoadConfiguration();
   OobeScreenWaiter(NetworkScreenView::kScreenId).Wait();
 
@@ -258,41 +239,42 @@ IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestSwitchLanguageIME) {
 }
 
 // Check that configuration lets correctly start Demo mode setup.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestEnableDemoMode) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestEnableDemoMode) {
   LoadConfiguration();
   OobeScreenWaiter(DemoPreferencesScreenView::kScreenId).Wait();
 }
 
 // Check that configuration lets correctly pass through demo preferences.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestDemoModePreferences) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestDemoModePreferences) {
   LoadConfiguration();
   OobeScreenWaiter(NetworkScreenView::kScreenId).Wait();
 }
 
 // Check that configuration lets correctly use offline demo mode on network
 // screen.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestDemoModeOfflineNetwork) {
-  SimulateOfflineEnvironment();
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest,
+                       MAYBE_TestDemoModeOfflineNetwork) {
   LoadConfiguration();
   OobeScreenWaiter(DemoPreferencesScreenView::kScreenId).Wait();
+  SimulateOfflineEnvironment();
   OobeScreenWaiter(EulaView::kScreenId).Wait();
 }
 
 // Check that configuration lets correctly use offline demo mode on EULA
 // screen.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestDemoModeAcceptEula) {
-  SimulateOfflineEnvironment();
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestDemoModeAcceptEula) {
   LoadConfiguration();
   OobeScreenWaiter(DemoPreferencesScreenView::kScreenId).Wait();
+  SimulateOfflineEnvironment();
   OobeScreenWaiter(ArcTermsOfServiceScreenView::kScreenId).Wait();
 }
 
 // Check that configuration lets correctly use offline demo mode on ARC++ ToS
 // screen.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestDemoModeAcceptArcTos) {
-  SimulateOfflineEnvironment();
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestDemoModeAcceptArcTos) {
   LoadConfiguration();
   OobeScreenWaiter(DemoPreferencesScreenView::kScreenId).Wait();
+  SimulateOfflineEnvironment();
 
   test::OobeJS().Evaluate(
       "login.ArcTermsOfServiceScreen.setTosForTesting('Test "
@@ -305,13 +287,14 @@ IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestDemoModeAcceptArcTos) {
 }
 
 // Check that configuration lets correctly select a network by GUID.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestSelectNetwork) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestSelectNetwork) {
   LoadConfiguration();
   OobeScreenWaiter(EulaView::kScreenId).Wait();
 }
 
 // Check that configuration would proceed if there is a connected network.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestSelectConnectedNetwork) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest,
+                       MAYBE_TestSelectConnectedNetwork) {
   LoadConfiguration();
   OobeScreenWaiter(EulaView::kScreenId).Wait();
 }
@@ -324,7 +307,7 @@ IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestConnectedNetworkNoWelcome) {
 }
 
 // Check that when configuration has ONC and EULA, we get to update screen.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestAcceptEula) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestAcceptEula) {
   update_engine::StatusResult status;
   status.set_current_operation(update_engine::Operation::DOWNLOADING);
   status.set_progress(0.1);
@@ -336,7 +319,7 @@ IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestAcceptEula) {
 
 // Check that when configuration has requisition, it gets applied at the
 // beginning.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, TestDeviceRequisition) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTest, MAYBE_TestDeviceRequisition) {
   LoadConfiguration();
   OobeScreenWaiter(EulaView::kScreenId).Wait();
   auto* policy_manager = g_browser_process->platform_part()
@@ -374,7 +357,7 @@ IN_PROC_BROWSER_TEST_F(OobeConfigurationTestNoHID, TestShowHID) {
 
 // Check that HID detection screen is really skipped and rest of configuration
 // is applied.
-IN_PROC_BROWSER_TEST_F(OobeConfigurationTestNoHID, TestSkipHIDDetection) {
+IN_PROC_BROWSER_TEST_F(OobeConfigurationTestNoHID, MAYBE_TestSkipHIDDetection) {
   LoadConfiguration();
   OobeScreenWaiter(NetworkScreenView::kScreenId).Wait();
 }