Tidy PluginVmLauncherViewBrowserTest

This CL tidies up PluginVmLauncherViewBrowserTest, removing the use of
inheritance to observe state changes and instead allowing the class
under test to have a callback which it calls upon completion. The newly
added functions will also be used for an upcoming autotest private API,
allowing the installer to be run from tast.

Bug: 1006664
Change-Id: I3abb2d0fd08d0988c140dbb564c70a02929d1ad8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863063Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706337}

chrome/browser/ui/views/plugin_vm/plugin_vm_launcher_view.cc

@@ -170,6 +170,11 @@ PluginVmLauncherView::PluginVmLauncherView(Profile* profile)
   layout->SetFlexForView(lower_container_view, 1, true);
 }
 
+// static
+PluginVmLauncherView* PluginVmLauncherView::GetActiveViewForTesting() {
+  return g_plugin_vm_launcher_view;
+}
+
 int PluginVmLauncherView::GetDialogButtons() const {
   switch (state_) {
     case State::START_DOWNLOADING:
@@ -364,6 +369,11 @@ base::string16 PluginVmLauncherView::GetMessage() const {
   }
 }
 
+void PluginVmLauncherView::SetFinishedCallbackForTesting(
+    base::OnceCallback<void(bool success)> callback) {
+  finished_callback_for_testing_ = std::move(callback);
+}
+
 PluginVmLauncherView::~PluginVmLauncherView() {
   plugin_vm_image_manager_->RemoveObserver();
   g_plugin_vm_launcher_view = nullptr;
@@ -407,6 +417,12 @@ void PluginVmLauncherView::OnStateUpdated() {
 
   DialogModelChanged();
   GetWidget()->GetRootView()->Layout();
+
+  if (state_ == State::FINISHED || state_ == State::ERROR ||
+      state_ == State::NOT_ALLOWED) {
+    if (finished_callback_for_testing_)
+      std::move(finished_callback_for_testing_).Run(state_ == State::FINISHED);
+  }
 }
 
 base::string16 PluginVmLauncherView::GetDownloadProgressMessage(

chrome/browser/ui/views/plugin_vm/plugin_vm_launcher_view.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_PLUGIN_VM_PLUGIN_VM_LAUNCHER_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_PLUGIN_VM_PLUGIN_VM_LAUNCHER_VIEW_H_
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_image_manager.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
@@ -26,6 +27,8 @@ class PluginVmLauncherView : public views::BubbleDialogDelegateView,
  public:
   explicit PluginVmLauncherView(Profile* profile);
 
+  static PluginVmLauncherView* GetActiveViewForTesting();
+
   // views::BubbleDialogDelegateView implementation.
   int GetDialogButtons() const override;
   base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
@@ -52,7 +55,10 @@ class PluginVmLauncherView : public views::BubbleDialogDelegateView,
   base::string16 GetBigMessage() const;
   base::string16 GetMessage() const;
 
- protected:
+  void SetFinishedCallbackForTesting(
+      base::OnceCallback<void(bool success)> callback);
+
+ private:
   enum class State {
     START_DOWNLOADING,  // PluginVm image downloading should be started.
     DOWNLOADING,        // PluginVm image downloading is in progress.
@@ -62,14 +68,12 @@ class PluginVmLauncherView : public views::BubbleDialogDelegateView,
     NOT_ALLOWED,        // PluginVm is disallowed on the device.
   };
 
-  State state_ = State::START_DOWNLOADING;
-
   ~PluginVmLauncherView() override;
-  virtual void OnStateUpdated();
+
+  void OnStateUpdated();
   // views::BubbleDialogDelegateView implementation.
   void AddedToWidget() override;
 
- private:
   base::string16 GetDownloadProgressMessage(uint64_t downlaoded_bytes,
                                             int64_t content_length) const;
   // Updates the progress bar and shows a time left message if available.
@@ -92,6 +96,10 @@ class PluginVmLauncherView : public views::BubbleDialogDelegateView,
   views::ImageView* big_image_ = nullptr;
   base::TimeTicks setup_start_tick_;
 
+  State state_ = State::START_DOWNLOADING;
+
+  base::OnceCallback<void(bool success)> finished_callback_for_testing_;
+
   DISALLOW_COPY_AND_ASSIGN(PluginVmLauncherView);
 };
 

chrome/browser/ui/views/plugin_vm/plugin_vm_launcher_view_browsertest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_metrics_util.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_test_helper.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/scoped_testing_cros_settings.h"
@@ -49,52 +50,8 @@ const char kJpgFileHash[] =
 
 }  // namespace
 
-class PluginVmLauncherViewForTesting : public PluginVmLauncherView {
- public:
-  explicit PluginVmLauncherViewForTesting(Profile* profile)
-      : PluginVmLauncherView(profile) {}
-
-  void AddSetupIsFinishedCallbackForTesting(base::RepeatingClosure callback) {
-    setup_is_finished_callback_for_testing_ = callback;
-  }
-
- private:
-  base::RepeatingClosure setup_is_finished_callback_for_testing_;
-
-  void OnStateUpdated() override {
-    PluginVmLauncherView::OnStateUpdated();
-
-    if (state_ == State::FINISHED || state_ == State::ERROR ||
-        state_ == State::NOT_ALLOWED) {
-      if (setup_is_finished_callback_for_testing_)
-        setup_is_finished_callback_for_testing_.Run();
-    }
-  }
-};
-
 class PluginVmLauncherViewBrowserTest : public DialogBrowserTest {
  public:
-  class SetupObserver {
-   public:
-    void OnSetupFinished() {
-      if (closure_) {
-        base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                                      std::move(closure_));
-      }
-    }
-
-    void WaitForSetupToFinish() {
-      base::RunLoop run_loop;
-      closure_ = run_loop.QuitClosure();
-      run_loop.Run();
-
-      content::RunAllTasksUntilIdle();
-    }
-
-   private:
-    base::OnceClosure closure_;
-  };
-
   PluginVmLauncherViewBrowserTest() = default;
 
   void SetUpOnMainThread() override {
@@ -110,11 +67,8 @@ class PluginVmLauncherViewBrowserTest : public DialogBrowserTest {
 
   // DialogBrowserTest:
   void ShowUi(const std::string& name) override {
-    view_ = new PluginVmLauncherViewForTesting(browser()->profile());
-    setup_observer_ = new SetupObserver();
-    view_->AddSetupIsFinishedCallbackForTesting(base::BindRepeating(
-        &SetupObserver::OnSetupFinished, base::Unretained(setup_observer_)));
-    views::DialogDelegate::CreateDialogWidget(view_, nullptr, nullptr);
+    plugin_vm::ShowPluginVmLauncherView(browser()->profile());
+    view_ = PluginVmLauncherView::GetActiveViewForTesting();
   }
 
  protected:
@@ -154,6 +108,16 @@ class PluginVmLauncherViewBrowserTest : public DialogBrowserTest {
         l10n_util::GetStringUTF16(IDS_PLUGIN_VM_LAUNCHER_NOT_ALLOWED_MESSAGE));
   }
 
+  void WaitForSetupToFinish() {
+    base::RunLoop run_loop;
+    view_->SetFinishedCallbackForTesting(
+        base::BindOnce(&PluginVmLauncherViewBrowserTest::OnSetupFinished,
+                       run_loop.QuitClosure()));
+
+    run_loop.Run();
+    content::RunAllTasksUntilIdle();
+  }
+
   void CheckSetupFailed() {
     EXPECT_TRUE(HasAcceptButton());
     EXPECT_TRUE(HasCancelButton());
@@ -176,8 +140,7 @@ class PluginVmLauncherViewBrowserTest : public DialogBrowserTest {
   chromeos::ScopedStubInstallAttributes scoped_stub_install_attributes_;
 
   std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
-  PluginVmLauncherViewForTesting* view_;
-  SetupObserver* setup_observer_;
+  PluginVmLauncherView* view_;
   std::unique_ptr<base::HistogramTester> histogram_tester_;
   chromeos::FakeConciergeClient* fake_concierge_client_;
 
@@ -206,6 +169,11 @@ class PluginVmLauncherViewBrowserTest : public DialogBrowserTest {
         std::move(user_manager));
   }
 
+  static void OnSetupFinished(base::OnceClosure quit_closure, bool success) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  std::move(quit_closure));
+  }
+
   DISALLOW_COPY_AND_ASSIGN(PluginVmLauncherViewBrowserTest);
 };
 
@@ -233,7 +201,7 @@ IN_PROC_BROWSER_TEST_F(PluginVmLauncherViewBrowserTestWithFeatureEnabled,
   ShowUi("default");
   EXPECT_NE(nullptr, view_);
 
-  setup_observer_->WaitForSetupToFinish();
+  WaitForSetupToFinish();
 
   CheckSetupIsFinishedSuccessfully();
 
@@ -252,7 +220,7 @@ IN_PROC_BROWSER_TEST_F(PluginVmLauncherViewBrowserTestWithFeatureEnabled,
   ShowUi("default");
   EXPECT_NE(nullptr, view_);
 
-  setup_observer_->WaitForSetupToFinish();
+  WaitForSetupToFinish();
 
   CheckSetupFailed();
 
@@ -270,7 +238,7 @@ IN_PROC_BROWSER_TEST_F(PluginVmLauncherViewBrowserTestWithFeatureEnabled,
   ShowUi("default");
   EXPECT_NE(nullptr, view_);
 
-  setup_observer_->WaitForSetupToFinish();
+  WaitForSetupToFinish();
 
   CheckSetupFailed();
 
@@ -289,7 +257,7 @@ IN_PROC_BROWSER_TEST_F(PluginVmLauncherViewBrowserTestWithFeatureEnabled,
   ShowUi("default");
   EXPECT_NE(nullptr, view_);
 
-  setup_observer_->WaitForSetupToFinish();
+  WaitForSetupToFinish();
 
   CheckSetupFailed();
 
@@ -300,7 +268,7 @@ IN_PROC_BROWSER_TEST_F(PluginVmLauncherViewBrowserTestWithFeatureEnabled,
   // Retry button clicked to retry the download.
   view_->GetDialogClientView()->AcceptWindow();
 
-  setup_observer_->WaitForSetupToFinish();
+  WaitForSetupToFinish();
 
   CheckSetupIsFinishedSuccessfully();