Reland "First browser tests for upgrader dialog"

This reverts commit 31a96092.

Reason for revert: Browser test failure only happens for
is_chrome_branded build. Disabled that case only.

Original change's description:
> 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/+/2135871
> Reviewed-by: Owen Min <zmin@chromium.org>
> Commit-Queue: Owen Min <zmin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#756282}

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1024693
Change-Id: I859ced5cc37e9f3248fb9fba4f793db917f8a5cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136981
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756606}

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

@@ -130,3 +130,32 @@ 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,6 +15,10 @@
 
 class CrostiniBrowserTestChromeBrowserMainExtraParts;
 
+namespace content {
+class WebContents;
+}
+
 // Common base for Crostini dialog broswer tests. Allows tests to set network
 // connection type.
 class CrostiniDialogBrowserTest : public DialogBrowserTest {
@@ -32,6 +36,8 @@ 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,25 +24,6 @@
 #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:
@@ -153,8 +134,8 @@ IN_PROC_BROWSER_TEST_F(CrostiniUpdateComponentViewBrowserTest,
     Browser* terminal_browser = web_app::FindSystemWebAppBrowser(
         browser()->profile(), web_app::SystemAppType::TERMINAL);
     CHECK_NE(nullptr, terminal_browser);
-    LoadFinishedWaiter(terminal_browser->tab_strip_model()->GetWebContentsAt(0))
-        .Wait();
+    WaitForLoadFinished(
+        terminal_browser->tab_strip_model()->GetWebContentsAt(0));
   }
 
   ExpectView();

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

@@ -46,7 +46,11 @@ CrostiniUpgraderDialog::CrostiniUpgraderDialog(
       only_run_launch_closure_on_restart_(only_run_launch_closure_on_restart),
       launch_closure_{std::move(launch_closure)} {}
 
-CrostiniUpgraderDialog::~CrostiniUpgraderDialog() = default;
+CrostiniUpgraderDialog::~CrostiniUpgraderDialog() {
+  if (deletion_closure_for_testing_) {
+    std::move(deletion_closure_for_testing_).Run();
+  }
+}
 
 void CrostiniUpgraderDialog::GetDialogSize(gfx::Size* size) const {
   size->SetSize(::kDialogWidth, ::kDialogHeight);
@@ -65,10 +69,19 @@ 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,6 +22,9 @@ 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);
@@ -37,10 +40,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 "build/branding_buildflags.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"
+
+#if !BUILDFLAG(GOOGLE_CHROME_BRANDING)
+
+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);
+}
+
+#endif  // !BUILDFLAG(GOOGLE_CHROME_BRANDING)

chrome/test/BUILD.gn

@@ -2426,6 +2426,7 @@ 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",