Revert "First browser tests for upgrader dialog"

This reverts commit 54171d06.

Reason for revert: crbug/1067636 CrostiniUpgraderDialogBrowserTest.ShowsOnAppLaunch failed on linux-chromeos-chrome

Original change's description:
> First browser tests for upgrader dialog
> 
> These tests cover the correct behavior on app launch.
> 
> Bug: 1024693
> Change-Id: I9808e7492e696f8fea89bf5c7eba49439c9fc5aa
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135091
> Commit-Queue: Nicholas Verne <nverne@chromium.org>
> Reviewed-by: Nic Hollingum <hollingum@google.com>
> Cr-Commit-Position: refs/heads/master@{#756193}

TBR=nverne@chromium.org,hollingum@google.com

Change-Id: I520ae534296f744c59f29aca7cbdaada5dc94327
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1024693
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135871Reviewed-by: Owen Min <zmin@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756282}

chrome/browser/ui/views/crostini/crostini_browser_test_util.cc

@@ -130,32 +130,3 @@ void CrostiniDialogBrowserTest::UnregisterTermina() {
           component_updater::CrOSComponentManager::Error::INSTALL_FAILURE,
           base::FilePath(), base::FilePath()));
 }
-
-class WebContentsWaiter : public content::WebContentsObserver {
- public:
-  enum Operation { LOAD };  // Add other operations as required.
-  explicit WebContentsWaiter(content::WebContents* contents,
-                             Operation operation)
-      : content::WebContentsObserver(contents), operation_(operation) {}
-
-  ~WebContentsWaiter() override = default;
-
-  void Wait() { run_loop_.Run(); }
-
-  // content::WebContentsObserver:
-  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
-                     const GURL& validated_url) override {
-    if (operation_ == LOAD) {
-      run_loop_.Quit();
-    }
-  }
-
- private:
-  base::RunLoop run_loop_;
-  Operation operation_;
-};
-
-void CrostiniDialogBrowserTest::WaitForLoadFinished(
-    content::WebContents* contents) {
-  WebContentsWaiter(contents, WebContentsWaiter::LOAD).Wait();
-}

chrome/browser/ui/views/crostini/crostini_browser_test_util.h

@@ -15,10 +15,6 @@
 
 class CrostiniBrowserTestChromeBrowserMainExtraParts;
 
-namespace content {
-class WebContents;
-}
-
 // Common base for Crostini dialog broswer tests. Allows tests to set network
 // connection type.
 class CrostiniDialogBrowserTest : public DialogBrowserTest {
@@ -36,8 +32,6 @@ class CrostiniDialogBrowserTest : public DialogBrowserTest {
 
   void UnregisterTermina();
 
-  void WaitForLoadFinished(content::WebContents* contents);
-
  protected:
   const bool register_termina_;
 

chrome/browser/ui/views/crostini/crostini_update_component_view_browsertest.cc

@@ -24,6 +24,25 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+class LoadFinishedWaiter : public content::WebContentsObserver {
+ public:
+  explicit LoadFinishedWaiter(content::WebContents* contents)
+      : content::WebContentsObserver(contents) {}
+
+  ~LoadFinishedWaiter() override = default;
+
+  void Wait() { run_loop_.Run(); }
+
+  // content::WebContentsObserver:
+  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
+                     const GURL& validated_url) override {
+    run_loop_.Quit();
+  }
+
+ private:
+  base::RunLoop run_loop_;
+};
+
 class CrostiniUpdateComponentViewBrowserTest
     : public CrostiniDialogBrowserTest {
  public:
@@ -134,8 +153,8 @@ IN_PROC_BROWSER_TEST_F(CrostiniUpdateComponentViewBrowserTest,
     Browser* terminal_browser = web_app::FindSystemWebAppBrowser(
         browser()->profile(), web_app::SystemAppType::TERMINAL);
     CHECK_NE(nullptr, terminal_browser);
-    WaitForLoadFinished(
-        terminal_browser->tab_strip_model()->GetWebContentsAt(0));
+    LoadFinishedWaiter(terminal_browser->tab_strip_model()->GetWebContentsAt(0))
+        .Wait();
   }
 
   ExpectView();

chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog.cc

@@ -46,11 +46,7 @@ CrostiniUpgraderDialog::CrostiniUpgraderDialog(
       only_run_launch_closure_on_restart_(only_run_launch_closure_on_restart),
       launch_closure_{std::move(launch_closure)} {}
 
-CrostiniUpgraderDialog::~CrostiniUpgraderDialog() {
-  if (deletion_closure_for_testing_) {
-    std::move(deletion_closure_for_testing_).Run();
-  }
-}
+CrostiniUpgraderDialog::~CrostiniUpgraderDialog() = default;
 
 void CrostiniUpgraderDialog::GetDialogSize(gfx::Size* size) const {
   size->SetSize(::kDialogWidth, ::kDialogHeight);
@@ -69,19 +65,10 @@ void CrostiniUpgraderDialog::AdjustWidgetInitParams(
   params->z_order = ui::ZOrderLevel::kNormal;
 }
 
-void CrostiniUpgraderDialog::SetDeletionClosureForTesting(
-    base::OnceClosure deletion_closure_for_testing) {
-  deletion_closure_for_testing_ = std::move(deletion_closure_for_testing);
-}
-
 bool CrostiniUpgraderDialog::CanCloseDialog() const {
   // TODO(929571): If other WebUI Dialogs also need to let the WebUI control
   // closing logic, we should find a more general solution.
 
-  if (deletion_closure_for_testing_) {
-    // Running in a test.
-    return true;
-  }
   // Disallow closing without WebUI consent.
   return upgrader_ui_ == nullptr || upgrader_ui_->can_close();
 }

chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog.h

@@ -22,9 +22,6 @@ class CrostiniUpgraderDialog : public SystemWebDialogDelegate {
   static void Show(base::OnceClosure launch_closure,
                    bool only_run_launch_closure_on_restart = false);
 
-  void SetDeletionClosureForTesting(
-      base::OnceClosure deletion_closure_for_testing);
-
  private:
   explicit CrostiniUpgraderDialog(base::OnceClosure launch_closure,
                                   bool only_run_launch_closure_on_restart);
@@ -40,10 +37,10 @@ class CrostiniUpgraderDialog : public SystemWebDialogDelegate {
   void OnCloseContents(content::WebContents* source,
                        bool* out_close_dialog) override;
 
+ private:
   CrostiniUpgraderUI* upgrader_ui_ = nullptr;  // Not owned.
   const bool only_run_launch_closure_on_restart_;
   base::OnceClosure launch_closure_;
-  base::OnceClosure deletion_closure_for_testing_;
 };
 
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog_browsertest.cc

-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog.h"
-
-#include "base/feature_list.h"
-#include "base/metrics/histogram_base.h"
-#include "base/run_loop.h"
-#include "base/test/metrics/histogram_tester.h"
-#include "chrome/browser/chromeos/crostini/crostini_manager.h"
-#include "chrome/browser/chromeos/crostini/crostini_test_helper.h"
-#include "chrome/browser/chromeos/crostini/crostini_util.h"
-#include "chrome/browser/chromeos/guest_os/guest_os_registry_service.h"
-#include "chrome/browser/chromeos/guest_os/guest_os_registry_service_factory.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/views/crostini/crostini_browser_test_util.h"
-#include "chrome/browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader.mojom.h"
-#include "chrome/common/webui_url_constants.h"
-#include "chromeos/dbus/cicerone/cicerone_service.pb.h"
-#include "content/public/browser/web_ui.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-constexpr char kDesktopFileId[] = "test_app";
-constexpr int kDisplayId = 0;
-
-class CrostiniUpgraderDialogBrowserTest : public CrostiniDialogBrowserTest {
- public:
-  CrostiniUpgraderDialogBrowserTest()
-      : CrostiniDialogBrowserTest(true /*register_termina*/),
-        app_id_(crostini::CrostiniTestHelper::GenerateAppId(kDesktopFileId)) {}
-
-  // DialogBrowserTest:
-  void ShowUi(const std::string& name) override {
-    chromeos::CrostiniUpgraderDialog::Show(base::DoNothing(), false);
-  }
-
-  chromeos::CrostiniUpgraderDialog* GetCrostiniUpgraderDialog() {
-    auto url = GURL{chrome::kChromeUICrostiniUpgraderUrl};
-    return reinterpret_cast<chromeos::CrostiniUpgraderDialog*>(
-        chromeos::SystemWebDialogDelegate::FindInstance(url.spec()));
-  }
-
-  void SafelyCloseDialog() {
-    auto* upgrader_dialog = GetCrostiniUpgraderDialog();
-
-    // Make sure the WebUI has launches sufficiently. Closing immediately would
-    // miss breakages in the underlying plumbing.
-    auto* web_contents = upgrader_dialog->GetWebUIForTest()->GetWebContents();
-    WaitForLoadFinished(web_contents);
-
-    // Now there should be enough WebUI hooked up to close properly.
-    base::RunLoop run_loop;
-    upgrader_dialog->SetDeletionClosureForTesting(run_loop.QuitClosure());
-    upgrader_dialog->Close();
-    run_loop.Run();
-  }
-
-  void ExpectDialog() {
-    // A new Widget was created in ShowUi() or since the last VerifyUi().
-    EXPECT_TRUE(crostini_manager()->GetCrostiniDialogStatus(
-        crostini::DialogType::UPGRADER));
-
-    EXPECT_NE(nullptr, GetCrostiniUpgraderDialog());
-  }
-
-  void ExpectNoDialog() {
-    // No new Widget was created in ShowUi() or since the last VerifyUi().
-    EXPECT_FALSE(crostini_manager()->GetCrostiniDialogStatus(
-        crostini::DialogType::UPGRADER));
-    // Our dialog has really been deleted.
-    EXPECT_EQ(nullptr, GetCrostiniUpgraderDialog());
-  }
-
-  void RegisterApp() {
-    vm_tools::apps::ApplicationList app_list =
-        crostini::CrostiniTestHelper::BasicAppList(
-            kDesktopFileId, crostini::kCrostiniDefaultVmName,
-            crostini::kCrostiniDefaultContainerName);
-    guest_os::GuestOsRegistryServiceFactory::GetForProfile(browser()->profile())
-        ->UpdateApplicationList(app_list);
-  }
-
-  void DowngradeOSRelease() {
-    vm_tools::cicerone::OsRelease os_release;
-    os_release.set_id("debian");
-    os_release.set_version_id("9");
-    auto container_id = crostini::DefaultContainerId();
-    crostini_manager()->SetContainerOsRelease(
-        container_id.vm_name, container_id.container_name, os_release);
-  }
-
-  const std::string& app_id() const { return app_id_; }
-
-  crostini::CrostiniManager* crostini_manager() {
-    return crostini::CrostiniManager::GetForProfile(browser()->profile());
-  }
-
- private:
-  std::string app_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(CrostiniUpgraderDialogBrowserTest);
-};
-
-IN_PROC_BROWSER_TEST_F(CrostiniUpgraderDialogBrowserTest,
-                       NoDialogOnNormalStartup) {
-  base::HistogramTester histogram_tester;
-  RegisterApp();
-
-  crostini::LaunchCrostiniApp(browser()->profile(), app_id(), kDisplayId);
-  ExpectNoDialog();
-}
-
-IN_PROC_BROWSER_TEST_F(CrostiniUpgraderDialogBrowserTest, ShowsOnAppLaunch) {
-  base::HistogramTester histogram_tester;
-
-  DowngradeOSRelease();
-  RegisterApp();
-
-  crostini::LaunchCrostiniApp(browser()->profile(), app_id(), kDisplayId);
-  ExpectDialog();
-
-  SafelyCloseDialog();
-  ExpectNoDialog();
-
-  // Once only - second time we launch an app, the dialog should not appear.
-  crostini::LaunchCrostiniApp(browser()->profile(), app_id(), kDisplayId);
-  ExpectNoDialog();
-
-  histogram_tester.ExpectUniqueSample(
-      crostini::kUpgradeDialogEventHistogram,
-      static_cast<base::HistogramBase::Sample>(
-          crostini::UpgradeDialogEvent::kDialogShown),
-      1);
-}

chrome/test/BUILD.gn

@@ -2425,7 +2425,6 @@ if (!is_android) {
         "../browser/ui/web_applications/web_app_guest_session_browsertest_chromeos.cc",
         "../browser/ui/webui/chromeos/add_supervision/add_supervision_metrics_recorder_browsertest.cc",
         "../browser/ui/webui/chromeos/add_supervision/add_supervision_ui_browsertest.cc",
-        "../browser/ui/webui/chromeos/crostini_upgrader/crostini_upgrader_dialog_browsertest.cc",
         "../browser/ui/webui/chromeos/login/discover/discover_browser_test.cc",
         "../browser/ui/webui/chromeos/login/discover/discover_browser_test.h",
         "../browser/ui/webui/chromeos/login/discover/modules/discover_module_launch_help_app_test.cc",