iwa: Add observer support to IsolatedWebAppInstallerModel
This adds an Observer interface to IsolatedWebAppInstallerModel, and
makes IsolatedWebAppInstallerViewController implement it. This allows
tests to also register as model observers so they can wait for events
in the installer.
This also updates IsolatedWebAppInstallerViewController unit tests
to use the model observer, which makes them more robust and clear.
Bug: 1479140
Change-Id: I9ad538b847bec963fe1fcd4cb9a44c668392900c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5208217
Reviewed-by: Zelin Liu <zelin@chromium.org>
Commit-Queue: Zelin Liu <zelin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1249603}
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 074a0d6..3ee14a1 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -6929,6 +6929,8 @@
"views/payments/test_chrome_payment_request_delegate.h",
"views/payments/test_secure_payment_confirmation_payment_request_delegate.cc",
"views/payments/test_secure_payment_confirmation_payment_request_delegate.h",
+ "views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.cc",
+ "views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.h",
"views/webauthn/authenticator_request_dialog_view_test_api.cc",
"views/webauthn/authenticator_request_dialog_view_test_api.h",
]
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.cc b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.cc
index 0a241b4..a74f8ce 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.cc
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.cc
@@ -43,10 +43,22 @@
const base::FilePath& bundle_path)
: bundle_path_(bundle_path), step_(Step::kDisabled) {}
+void IsolatedWebAppInstallerModel::AddObserver(Observer* observer) {
+ observers_.AddObserver(observer);
+}
+
+void IsolatedWebAppInstallerModel::RemoveObserver(Observer* observer) {
+ observers_.RemoveObserver(observer);
+}
+
IsolatedWebAppInstallerModel::~IsolatedWebAppInstallerModel() = default;
void IsolatedWebAppInstallerModel::SetStep(Step step) {
step_ = step;
+
+ for (Observer& observer : observers_) {
+ observer.OnStepChanged();
+ }
}
void IsolatedWebAppInstallerModel::SetSignedWebBundleMetadata(
@@ -56,6 +68,10 @@
void IsolatedWebAppInstallerModel::SetDialog(std::optional<Dialog> dialog) {
dialog_ = dialog;
+
+ for (Observer& observer : observers_) {
+ observer.OnChildDialogChanged();
+ }
}
} // namespace web_app
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h
index d83833b58..b2a7c32 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h
@@ -10,6 +10,7 @@
#include "base/files/file_path.h"
#include "base/functional/callback_forward.h"
+#include "base/observer_list.h"
#include "base/version.h"
#include "chrome/browser/web_applications/isolated_web_apps/signed_web_bundle_metadata.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
@@ -18,6 +19,12 @@
class IsolatedWebAppInstallerModel {
public:
+ class Observer : public base::CheckedObserver {
+ public:
+ virtual void OnStepChanged() = 0;
+ virtual void OnChildDialogChanged() = 0;
+ };
+
enum class Step {
kDisabled,
kGetMetadata,
@@ -63,6 +70,9 @@
explicit IsolatedWebAppInstallerModel(const base::FilePath& bundle_path);
~IsolatedWebAppInstallerModel();
+ void AddObserver(Observer* observer);
+ void RemoveObserver(Observer* observer);
+
const base::FilePath& bundle_path() { return bundle_path_; }
void SetStep(Step step);
@@ -77,6 +87,7 @@
const Dialog& dialog() { return dialog_.value(); }
private:
+ base::ObserverList<Observer> observers_;
base::FilePath bundle_path_;
Step step_;
std::optional<SignedWebBundleMetadata> bundle_metadata_;
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_browsertest.cc b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_browsertest.cc
index 5b67b46..b0814876 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_browsertest.cc
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_browsertest.cc
@@ -245,14 +245,13 @@
controller.Start(future.GetCallback(), base::DoNothing());
ASSERT_TRUE(future.Wait());
- model.SetStep(GetParam().step);
model.SetSignedWebBundleMetadata(CreateTestMetadata());
+ model.SetStep(GetParam().step);
controller.Show();
if (GetParam().dialog.has_value()) {
model.SetDialog(GetParam().dialog);
- controller.OnModelChanged();
}
}
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.cc b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.cc
index 50b5319..7e056b5 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.cc
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.cc
@@ -108,7 +108,6 @@
LOG(ERROR) << "Isolated Web App bundle installability check failed: "
<< invalid.error;
model_->SetDialog(IsolatedWebAppInstallerModel::BundleInvalidDialog{});
- controller_->OnModelChanged();
}
void operator()(const InstallabilityChecker::BundleInstallable& installable) {
@@ -124,7 +123,6 @@
}
model_->SetSignedWebBundleMetadata(installable.metadata);
model_->SetStep(IsolatedWebAppInstallerModel::Step::kShowMetadata);
- controller_->OnModelChanged();
}
void operator()(const InstallabilityChecker::BundleUpdatable& updatable) {
@@ -142,7 +140,6 @@
outdated.metadata.app_name(), outdated.metadata.version(),
outdated.installed_version});
}
- controller_->OnModelChanged();
}
void operator()(const InstallabilityChecker::ProfileShutdown&) {
@@ -170,10 +167,13 @@
CHECK(model_);
CHECK(web_app_provider_);
CHECK(pref_observer_);
+ model_->AddObserver(this);
}
IsolatedWebAppInstallerViewController::
- ~IsolatedWebAppInstallerViewController() = default;
+ ~IsolatedWebAppInstallerViewController() {
+ model_->RemoveObserver(this);
+}
void IsolatedWebAppInstallerViewController::Start(
base::OnceClosure initialized_callback,
@@ -269,7 +269,8 @@
CreateDialogDelegate(std::move(view));
dialog_delegate_ = dialog_delegate.get();
- OnModelChanged();
+ OnStepChanged();
+ OnChildDialogChanged();
views::Widget* widget =
views::DialogDelegate::CreateDialogWidget(std::move(dialog_delegate),
@@ -309,7 +310,6 @@
base::BindRepeating(&IsolatedWebAppInstallerViewController::
OnShowMetadataLearnMoreClicked,
base::Unretained(this))});
- OnModelChanged();
return false;
}
@@ -384,7 +384,6 @@
installability_checker_.reset();
}
}
- OnModelChanged();
if (!is_initialized_) {
is_initialized_ = true;
std::move(initialized_callback_).Run();
@@ -418,7 +417,6 @@
} else {
model_->SetDialog(IsolatedWebAppInstallerModel::InstallationFailedDialog{});
}
- OnModelChanged();
}
void IsolatedWebAppInstallerViewController::OnShowMetadataLearnMoreClicked() {
@@ -450,11 +448,10 @@
}
void IsolatedWebAppInstallerViewController::OnChildDialogAccepted() {
+ model_->SetDialog(std::nullopt);
switch (model_->step()) {
case IsolatedWebAppInstallerModel::Step::kShowMetadata: {
model_->SetStep(IsolatedWebAppInstallerModel::Step::kInstall);
- model_->SetDialog(std::nullopt);
- OnModelChanged();
callback_delayer_ = std::make_unique<CallbackDelayer>(
kInstallationMinimumDelay, kProgressBarPausePercentage,
@@ -475,7 +472,6 @@
case IsolatedWebAppInstallerModel::Step::kInstall:
// A child dialog on the install screen means the installation failed.
// Accepting the dialog corresponds to the Retry button.
- model_->SetDialog(std::nullopt);
installability_checker_.reset();
pref_observer_->Reset();
Start(base::DoNothing(), std::move(completion_callback_));
@@ -486,7 +482,7 @@
}
}
-void IsolatedWebAppInstallerViewController::OnModelChanged() {
+void IsolatedWebAppInstallerViewController::OnStepChanged() {
if (!view_) {
return;
}
@@ -526,7 +522,9 @@
view_->ShowInstallSuccessScreen(model_->bundle_metadata());
break;
}
+}
+void IsolatedWebAppInstallerViewController::OnChildDialogChanged() {
if (model_->has_dialog()) {
view_->ShowDialog(model_->dialog());
}
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.h b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.h
index 89fb808..b7cc8ecc 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.h
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller.h
@@ -13,6 +13,7 @@
#include "base/memory/weak_ptr.h"
#include "base/types/expected.h"
#include "chrome/browser/ui/views/web_apps/isolated_web_apps/installability_checker.h"
+#include "chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h"
#include "chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view.h"
#include "chrome/browser/ui/views/web_apps/isolated_web_apps/pref_observer.h"
#include "chrome/browser/web_applications/isolated_web_apps/install_isolated_web_app_command.h"
@@ -29,18 +30,18 @@
namespace web_app {
class CallbackDelayer;
-class IsolatedWebAppInstallerModel;
class WebAppProvider;
class IsolatedWebAppInstallerViewController
- : public IsolatedWebAppInstallerView::Delegate {
+ : public IsolatedWebAppInstallerModel::Observer,
+ public IsolatedWebAppInstallerView::Delegate {
public:
IsolatedWebAppInstallerViewController(
Profile* profile,
WebAppProvider* web_app_provider,
IsolatedWebAppInstallerModel* model,
std::unique_ptr<IsolatedWebAppsEnabledPrefObserver> pref_observer);
- virtual ~IsolatedWebAppInstallerViewController();
+ ~IsolatedWebAppInstallerViewController() override;
// Starts the installer state transition. |initialized_callback| will be
// called once the dialog is initialized and ready for display.
@@ -101,8 +102,9 @@
void OnChildDialogCanceled() override;
void OnChildDialogAccepted() override;
- // Updates the View to reflect the current state of the model.
- void OnModelChanged();
+ // `IsolatedWebAppInstallerModel::Observer`:
+ void OnStepChanged() override;
+ void OnChildDialogChanged() override;
std::unique_ptr<views::DialogDelegate> CreateDialogDelegate(
std::unique_ptr<views::View> contents_view);
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller_unittest.cc b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller_unittest.cc
index 07efe0e..021575b 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller_unittest.cc
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_controller_unittest.cc
@@ -21,6 +21,7 @@
#include "chrome/browser/apps/app_service/app_launch_params.h"
#include "chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h"
#include "chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view.h"
+#include "chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.h"
#include "chrome/browser/ui/web_applications/test/isolated_web_app_builder.h"
#include "chrome/browser/ui/web_applications/test/isolated_web_app_test_utils.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_location.h"
@@ -73,10 +74,10 @@
using ::testing::Exactly;
using ::testing::ExplainMatchResult;
using ::testing::Field;
-using ::testing::IgnoreResult;
using ::testing::Invoke;
using ::testing::Property;
using ::testing::VariantWith;
+using Step = IsolatedWebAppInstallerModel::Step;
constexpr base::StringPiece kIconPath = "/icon.png";
@@ -290,18 +291,16 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- base::test::TestFuture<void> callback;
EXPECT_CALL(view, UpdateGetMetadataProgress(_)).Times(AnyNumber());
EXPECT_CALL(view, ShowGetMetadataScreen());
EXPECT_CALL(
view, ShowMetadataScreen(WithMetadata("hoealecpbefphiclhampllbdbdpfmfpi",
- u"test app name", "7.7.7")))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ u"test app name", "7.7.7")));
controller.Start(base::DoNothing(), base::DoNothing());
- EXPECT_TRUE(callback.Wait());
- EXPECT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kShowMetadata);
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForStepChange(
+ Step::kShowMetadata);
}
TEST_F(IsolatedWebAppInstallerViewControllerTest,
@@ -323,19 +322,17 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- base::test::TestFuture<void> callback;
EXPECT_CALL(view, UpdateGetMetadataProgress(_)).Times(AnyNumber());
- EXPECT_CALL(view, ShowGetMetadataScreen()).Times(Exactly(2));
+ EXPECT_CALL(view, ShowGetMetadataScreen());
EXPECT_CALL(
view,
ShowDialog(
- VariantWith<IsolatedWebAppInstallerModel::BundleInvalidDialog>(_)))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ VariantWith<IsolatedWebAppInstallerModel::BundleInvalidDialog>(_)));
controller.Start(base::DoNothing(), base::DoNothing());
- EXPECT_TRUE(callback.Wait());
- EXPECT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kGetMetadata);
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForChildDialog();
+ EXPECT_EQ(model.step(), Step::kGetMetadata);
}
TEST_F(IsolatedWebAppInstallerViewControllerTest,
@@ -350,6 +347,8 @@
base::Version("2.0")));
IsolatedWebAppInstallerModel model(CreateBundlePath("test_bundle.swbn"));
+ model.SetStep(Step::kGetMetadata);
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -357,26 +356,26 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kGetMetadata);
-
- base::test::TestFuture<void> callback;
EXPECT_CALL(view, UpdateGetMetadataProgress(_)).Times(AnyNumber());
- EXPECT_CALL(view, ShowGetMetadataScreen()).Times(Exactly(2));
+ EXPECT_CALL(view, ShowGetMetadataScreen());
EXPECT_CALL(
view,
ShowDialog(
- VariantWith<IsolatedWebAppInstallerModel::BundleOutdatedDialog>(_)))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ VariantWith<IsolatedWebAppInstallerModel::BundleOutdatedDialog>(_)));
controller.Start(base::DoNothing(), base::DoNothing());
- EXPECT_TRUE(callback.Wait());
- EXPECT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kGetMetadata);
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForChildDialog();
+ EXPECT_EQ(model.step(), Step::kGetMetadata);
}
TEST_F(IsolatedWebAppInstallerViewControllerTest,
InstallButtonLaunchesConfirmationDialog) {
IsolatedWebAppInstallerModel model(CreateBundlePath("test_bundle.swbn"));
+ SignedWebBundleMetadata metadata = CreateMetadata(u"Test App", "0.0.1");
+ model.SetSignedWebBundleMetadata(metadata);
+ model.SetStep(Step::kShowMetadata);
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -384,27 +383,26 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- SignedWebBundleMetadata metadata = CreateMetadata(u"Test App", "0.0.1");
- model.SetSignedWebBundleMetadata(metadata);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kShowMetadata);
-
- base::test::TestFuture<void> callback;
- EXPECT_CALL(view, ShowMetadataScreen(metadata));
EXPECT_CALL(
view,
ShowDialog(
VariantWith<IsolatedWebAppInstallerModel::ConfirmInstallationDialog>(
- _)))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ _)));
controller.OnAccept();
- EXPECT_TRUE(callback.Wait());
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForChildDialog();
}
TEST_F(IsolatedWebAppInstallerViewControllerTest,
ConfirmationDialogMovesToInstallScreen) {
IsolatedWebAppInstallerModel model(CreateBundlePath("test_bundle.swbn"));
+ SignedWebBundleMetadata metadata = CreateMetadata(u"Test App", "0.0.1");
+ model.SetSignedWebBundleMetadata(metadata);
+ model.SetStep(Step::kShowMetadata);
+ model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
+ base::DoNothing()});
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -412,19 +410,12 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- SignedWebBundleMetadata metadata = CreateMetadata(u"Test App", "0.0.1");
- model.SetSignedWebBundleMetadata(metadata);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kShowMetadata);
- model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
- base::DoNothing()});
-
- base::test::TestFuture<void> callback;
- EXPECT_CALL(view, ShowInstallScreen(metadata))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ EXPECT_CALL(view, ShowInstallScreen(metadata));
controller.OnChildDialogAccepted();
- EXPECT_TRUE(callback.Wait());
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForStepChange(
+ Step::kInstall);
}
TEST_F(IsolatedWebAppInstallerViewControllerTest,
@@ -434,6 +425,14 @@
MockIconAndPageState(url_info, "1.0");
IsolatedWebAppInstallerModel model(bundle_path);
+ auto metadata = SignedWebBundleMetadata::CreateForTesting(
+ url_info, InstalledBundle(bundle_path), u"app name", base::Version("1.0"),
+ IconBitmaps());
+ model.SetSignedWebBundleMetadata(metadata);
+ model.SetStep(Step::kShowMetadata);
+ model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
+ base::DoNothing()});
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -441,23 +440,14 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- auto metadata = SignedWebBundleMetadata::CreateForTesting(
- url_info, InstalledBundle(bundle_path), u"app name", base::Version("1.0"),
- IconBitmaps());
- model.SetSignedWebBundleMetadata(metadata);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kShowMetadata);
- model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
- base::DoNothing()});
-
- base::test::TestFuture<void> callback;
EXPECT_CALL(view, UpdateInstallProgress(_)).Times(AnyNumber());
EXPECT_CALL(view, ShowInstallScreen(metadata));
- EXPECT_CALL(view, ShowInstallSuccessScreen(metadata))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ EXPECT_CALL(view, ShowInstallSuccessScreen(metadata));
controller.OnChildDialogAccepted();
- EXPECT_TRUE(callback.Wait());
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForStepChange(
+ Step::kInstallSuccess);
EXPECT_TRUE(
fake_provider()->registrar_unsafe().IsInstalled(url_info.app_id()));
}
@@ -468,6 +458,14 @@
MockIconAndPageState(url_info, "1.0");
IsolatedWebAppInstallerModel model(bundle_path);
+ auto metadata = SignedWebBundleMetadata::CreateForTesting(
+ url_info, InstalledBundle(bundle_path), u"app name", base::Version("1.0"),
+ IconBitmaps());
+ model.SetSignedWebBundleMetadata(metadata);
+ model.SetStep(Step::kShowMetadata);
+ model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
+ base::DoNothing()});
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -475,26 +473,21 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- auto metadata = SignedWebBundleMetadata::CreateForTesting(
- url_info, InstalledBundle(bundle_path), u"app name", base::Version("1.0"),
- IconBitmaps());
- model.SetSignedWebBundleMetadata(metadata);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kShowMetadata);
- model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
- base::DoNothing()});
-
- EXPECT_CALL(view, UpdateInstallProgress(_)).Times(AnyNumber());
EXPECT_CALL(view, ShowInstallScreen(metadata));
- EXPECT_CALL(view, ShowInstallSuccessScreen(metadata))
- .WillOnce(IgnoreResult(Invoke(
- &controller, &IsolatedWebAppInstallerViewController::OnAccept)));
+ EXPECT_CALL(view, UpdateInstallProgress(_)).Times(AnyNumber());
+ EXPECT_CALL(view, ShowInstallSuccessScreen(metadata));
+
+ controller.OnChildDialogAccepted();
+
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForStepChange(
+ Step::kInstallSuccess);
base::test::TestFuture<apps::AppLaunchParams, LaunchWebAppWindowSetting>
future;
static_cast<FakeWebAppUiManager*>(&fake_provider()->ui_manager())
->SetOnLaunchWebAppCallback(future.GetRepeatingCallback());
- controller.OnChildDialogAccepted();
+ controller.OnAccept();
EXPECT_EQ(future.Get<0>().app_id, metadata.app_id());
}
@@ -506,6 +499,14 @@
MockIconAndPageState(url_info, "1.0");
IsolatedWebAppInstallerModel model(bundle_path);
+ auto metadata = SignedWebBundleMetadata::CreateForTesting(
+ url_info, InstalledBundle(bundle_path), u"app name", base::Version("2.0"),
+ IconBitmaps());
+ model.SetSignedWebBundleMetadata(metadata);
+ model.SetStep(Step::kShowMetadata);
+ model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
+ base::DoNothing()});
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -513,27 +514,17 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- auto metadata = SignedWebBundleMetadata::CreateForTesting(
- url_info, InstalledBundle(bundle_path), u"app name", base::Version("2.0"),
- IconBitmaps());
- model.SetSignedWebBundleMetadata(metadata);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kShowMetadata);
- model.SetDialog(IsolatedWebAppInstallerModel::ConfirmInstallationDialog{
- base::DoNothing()});
-
- base::test::TestFuture<void> callback;
EXPECT_CALL(view, UpdateInstallProgress(_)).Times(AnyNumber());
- EXPECT_CALL(view, ShowInstallScreen(metadata)).Times(Exactly(2));
+ EXPECT_CALL(view, ShowInstallScreen(metadata));
EXPECT_CALL(
view,
ShowDialog(
VariantWith<IsolatedWebAppInstallerModel::InstallationFailedDialog>(
- _)))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ _)));
controller.OnChildDialogAccepted();
- EXPECT_TRUE(callback.Wait());
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForChildDialog();
EXPECT_FALSE(
fake_provider()->registrar_unsafe().IsInstalled(url_info.app_id()));
}
@@ -541,6 +532,11 @@
TEST_F(IsolatedWebAppInstallerViewControllerTest,
InstallationErrorRetryRestartsFlow) {
IsolatedWebAppInstallerModel model(CreateBundlePath("test_bundle.swbn"));
+ SignedWebBundleMetadata metadata = CreateMetadata(u"Test App", "0.0.1");
+ model.SetSignedWebBundleMetadata(metadata);
+ model.SetStep(Step::kInstall);
+ model.SetDialog(IsolatedWebAppInstallerModel::InstallationFailedDialog{});
+
auto pref_observer =
std::make_unique<FakeIsolatedWebAppsEnabledPrefObserver>(true);
IsolatedWebAppInstallerViewController controller(
@@ -548,21 +544,14 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
-
controller.completion_callback_ = base::DoNothing();
- SignedWebBundleMetadata metadata = CreateMetadata(u"Test App", "0.0.1");
- model.SetSignedWebBundleMetadata(metadata);
- model.SetStep(IsolatedWebAppInstallerModel::Step::kInstall);
- model.SetDialog(IsolatedWebAppInstallerModel::InstallationFailedDialog{});
-
- base::test::TestFuture<void> callback;
- EXPECT_CALL(view, ShowGetMetadataScreen())
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ EXPECT_CALL(view, ShowGetMetadataScreen());
controller.OnChildDialogAccepted();
- EXPECT_TRUE(callback.Wait());
+ TestIsolatedWebAppInstallerModelObserver(&model).WaitForStepChange(
+ Step::kGetMetadata);
}
#if BUILDFLAG(IS_CHROMEOS)
@@ -583,29 +572,22 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- base::test::TestFuture<void> callback;
EXPECT_CALL(view, UpdateGetMetadataProgress(_)).Times(AnyNumber());
EXPECT_CALL(view, ShowGetMetadataScreen());
EXPECT_CALL(
view, ShowMetadataScreen(WithMetadata("hoealecpbefphiclhampllbdbdpfmfpi",
- u"test app name", "7.7.7")))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ u"test app name", "7.7.7")));
controller.Start(base::DoNothing(), base::DoNothing());
- ASSERT_TRUE(callback.Wait());
- ASSERT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kShowMetadata);
+ TestIsolatedWebAppInstallerModelObserver model_observer(&model);
+ model_observer.WaitForStepChange(Step::kShowMetadata);
- callback.Clear();
-
- EXPECT_CALL(view, ShowDisabledScreen())
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ EXPECT_CALL(view, ShowDisabledScreen());
raw_pref_observer->UpdatePref(false);
- EXPECT_TRUE(callback.Wait());
-
- EXPECT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kDisabled);
+ model_observer.WaitForStepChange(Step::kDisabled);
}
TEST_F(IsolatedWebAppInstallerViewControllerTest,
@@ -624,29 +606,24 @@
testing::StrictMock<MockView> view;
controller.SetViewForTesting(&view);
- base::test::TestFuture<void> callback;
- EXPECT_CALL(view, ShowDisabledScreen())
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ EXPECT_CALL(view, ShowDisabledScreen());
controller.Start(base::DoNothing(), base::DoNothing());
- ASSERT_TRUE(callback.Wait());
- ASSERT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kDisabled);
+ TestIsolatedWebAppInstallerModelObserver model_observer(&model);
+ model_observer.WaitForStepChange(Step::kDisabled);
- callback.Clear();
+ ASSERT_EQ(model.step(), Step::kDisabled);
EXPECT_CALL(view, UpdateGetMetadataProgress(_)).Times(AnyNumber());
EXPECT_CALL(view, ShowGetMetadataScreen());
EXPECT_CALL(
view, ShowMetadataScreen(WithMetadata("hoealecpbefphiclhampllbdbdpfmfpi",
- u"test app name", "7.7.7")))
- .WillOnce(Invoke(&callback, &base::test::TestFuture<void>::SetValue));
+ u"test app name", "7.7.7")));
raw_pref_observer->UpdatePref(true);
- EXPECT_TRUE(callback.Wait());
-
- EXPECT_EQ(model.step(), IsolatedWebAppInstallerModel::Step::kShowMetadata);
+ model_observer.WaitForStepChange(Step::kShowMetadata);
}
#endif // BUILDFLAG(IS_CHROMEOS)
} // namespace web_app
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.cc b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.cc
index d84b38c..c9064782e 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.cc
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.cc
@@ -415,7 +415,8 @@
get_metadata_view_(MakeAndAddChildView<GetMetadataView>()),
show_metadata_view_(MakeAndAddChildView<ShowMetadataView>(delegate)),
install_view_(MakeAndAddChildView<InstallView>()),
- install_success_view_(MakeAndAddChildView<InstallSuccessView>()) {
+ install_success_view_(MakeAndAddChildView<InstallSuccessView>()),
+ dialog_visible_(false) {
SetUseDefaultFillLayout(true);
ShowChildView(nullptr);
}
@@ -476,7 +477,7 @@
absl::visit(
base::Overloaded{
[this](const IsolatedWebAppInstallerModel::BundleInvalidDialog&) {
- ShowDialog(
+ ShowChildDialog(
IDS_IWA_INSTALLER_VERIFICATION_ERROR_TITLE,
ui::DialogModelLabel(
IDS_IWA_INSTALLER_VERIFICATION_ERROR_SUBTITLE),
@@ -497,7 +498,7 @@
ui::DialogModelLabel::CreatePlainText(
base::UTF8ToUTF16(installed_version)),
});
- ShowDialog(
+ ShowChildDialog(
IDS_IWA_INSTALLER_ALREADY_INSTALLED_TITLE, subtitle,
CreateImageModelFromVector(vector_icons::kErrorOutlineIcon,
ui::kColorAlertMediumSeverityIcon),
@@ -515,7 +516,7 @@
ui::DialogModelLabel::CreatePlainText(
base::UTF8ToUTF16(installed_version)),
});
- ShowDialog(
+ ShowChildDialog(
IDS_IWA_INSTALLER_BUNDLE_OUTDATED_TITLE, subtitle,
CreateImageModelFromVector(vector_icons::kErrorOutlineIcon,
ui::kColorAlertMediumSeverityIcon),
@@ -528,14 +529,14 @@
ui::DialogModelLabel::CreateLink(
IDS_IWA_INSTALLER_CONFIRM_LEARN_MORE,
confirm_installation_dialog.learn_more_callback));
- ShowDialog(
+ ShowChildDialog(
IDS_IWA_INSTALLER_CONFIRM_TITLE, subtitle,
CreateImageModelFromVector(kSecurityIcon, ui::kColorAccent),
IDS_IWA_INSTALLER_CONFIRM_CONTINUE);
},
[this](
const IsolatedWebAppInstallerModel::InstallationFailedDialog&) {
- ShowDialog(
+ ShowChildDialog(
IDS_IWA_INSTALLER_INSTALL_FAILED_TITLE,
ui::DialogModelLabel(IDS_IWA_INSTALLER_INSTALL_FAILED_SUBTITLE),
CreateImageModelFromVector(vector_icons::kErrorOutlineIcon,
@@ -554,23 +555,27 @@
return gfx::Size(width, GetHeightForWidth(width));
}
-void IsolatedWebAppInstallerViewImpl::ShowDialog(
+void IsolatedWebAppInstallerViewImpl::ShowChildDialog(
int title,
const ui::DialogModelLabel& subtitle,
const ui::ImageModel& icon_model,
std::optional<int> ok_label) {
+ CHECK(!dialog_visible_);
+ dialog_visible_ = true;
+
ui::DialogModel::Builder dialog_model_builder;
dialog_model_builder
.SetInternalName(IsolatedWebAppInstallerView::kNestedDialogWidgetName)
.SetTitle(l10n_util::GetStringUTF16(title))
.AddParagraph(ui::DialogModelLabel(subtitle).set_is_secondary())
.DisableCloseOnDeactivate()
- .AddCancelButton(base::BindOnce(&Delegate::OnChildDialogCanceled,
- base::Unretained(delegate_)));
+ .AddCancelButton(base::BindOnce(
+ &IsolatedWebAppInstallerViewImpl::OnChildDialogCanceled,
+ base::Unretained(this)));
if (ok_label.has_value()) {
dialog_model_builder.AddOkButton(
- base::BindOnce(&Delegate::OnChildDialogAccepted,
- base::Unretained(delegate_)),
+ base::BindOnce(&IsolatedWebAppInstallerViewImpl::OnChildDialogAccepted,
+ base::Unretained(this)),
ui::DialogModel::Button::Params().SetLabel(
l10n_util::GetStringUTF16(ok_label.value())));
}
@@ -606,6 +611,16 @@
views::BubbleDialogDelegate::CreateBubble(std::move(bubble))->Show();
}
+void IsolatedWebAppInstallerViewImpl::OnChildDialogAccepted() {
+ dialog_visible_ = false;
+ delegate_->OnChildDialogAccepted();
+}
+
+void IsolatedWebAppInstallerViewImpl::OnChildDialogCanceled() {
+ dialog_visible_ = false;
+ delegate_->OnChildDialogCanceled();
+}
+
void IsolatedWebAppInstallerViewImpl::ShowChildView(views::View* view) {
for (views::View* child : children()) {
child->SetVisible(child == view);
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.h b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.h
index f60f46e..0963b72 100644
--- a/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.h
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_view_impl.h
@@ -64,10 +64,13 @@
return AddChildView(std::make_unique<T>(std::forward<Args>(args)...));
}
- void ShowDialog(int title,
- const ui::DialogModelLabel& subtitle,
- const ui::ImageModel& icon,
- std::optional<int> ok_label);
+ void ShowChildDialog(int title,
+ const ui::DialogModelLabel& subtitle,
+ const ui::ImageModel& icon,
+ std::optional<int> ok_label);
+
+ void OnChildDialogAccepted();
+ void OnChildDialogCanceled();
void ShowChildView(views::View* view);
@@ -78,6 +81,8 @@
raw_ptr<ShowMetadataView> show_metadata_view_;
raw_ptr<InstallView> install_view_;
raw_ptr<InstallSuccessView> install_success_view_;
+
+ bool dialog_visible_;
};
} // namespace web_app
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.cc b/chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.cc
new file mode 100644
index 0000000..147f5211
--- /dev/null
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.cc
@@ -0,0 +1,58 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.h"
+
+#include "base/test/test_future.h"
+
+namespace web_app {
+
+TestIsolatedWebAppInstallerModelObserver::
+ TestIsolatedWebAppInstallerModelObserver(
+ IsolatedWebAppInstallerModel* model)
+ : model_(model) {
+ model_->AddObserver(this);
+}
+
+TestIsolatedWebAppInstallerModelObserver::
+ ~TestIsolatedWebAppInstallerModelObserver() {
+ model_->RemoveObserver(this);
+}
+
+void TestIsolatedWebAppInstallerModelObserver::WaitForChildDialog() {
+ CHECK(!dialog_changed_callback_);
+ while (!model_->has_dialog()) {
+ base::test::TestFuture<void> future;
+ dialog_changed_callback_ = future.GetCallback();
+ CHECK(future.Wait());
+ }
+}
+
+void TestIsolatedWebAppInstallerModelObserver::WaitForStepChange(
+ IsolatedWebAppInstallerModel::Step step) {
+ while (model_->step() != step) {
+ WaitForStepChange();
+ }
+}
+
+void TestIsolatedWebAppInstallerModelObserver::WaitForStepChange() {
+ CHECK(!step_changed_callback_);
+ base::test::TestFuture<void> future;
+ step_changed_callback_ = future.GetCallback();
+ CHECK(future.Wait());
+}
+
+void TestIsolatedWebAppInstallerModelObserver::OnStepChanged() {
+ if (step_changed_callback_) {
+ std::move(step_changed_callback_).Run();
+ }
+}
+
+void TestIsolatedWebAppInstallerModelObserver::OnChildDialogChanged() {
+ if (dialog_changed_callback_) {
+ std::move(dialog_changed_callback_).Run();
+ }
+}
+
+} // namespace web_app
diff --git a/chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.h b/chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.h
new file mode 100644
index 0000000..aea2bb1
--- /dev/null
+++ b/chrome/browser/ui/views/web_apps/isolated_web_apps/test_isolated_web_app_installer_model_observer.h
@@ -0,0 +1,38 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_WEB_APPS_ISOLATED_WEB_APPS_TEST_ISOLATED_WEB_APP_INSTALLER_MODEL_OBSERVER_H_
+#define CHROME_BROWSER_UI_VIEWS_WEB_APPS_ISOLATED_WEB_APPS_TEST_ISOLATED_WEB_APP_INSTALLER_MODEL_OBSERVER_H_
+
+#include "base/functional/callback_forward.h"
+#include "chrome/browser/ui/views/web_apps/isolated_web_apps/isolated_web_app_installer_model.h"
+
+namespace web_app {
+
+class TestIsolatedWebAppInstallerModelObserver
+ : public IsolatedWebAppInstallerModel::Observer {
+ public:
+ explicit TestIsolatedWebAppInstallerModelObserver(
+ IsolatedWebAppInstallerModel* model);
+ ~TestIsolatedWebAppInstallerModelObserver() override;
+
+ void WaitForChildDialog();
+
+ void WaitForStepChange(IsolatedWebAppInstallerModel::Step step);
+
+ private:
+ void WaitForStepChange();
+
+ // `IsolatedWebAppInstallerModel::Observer`:
+ void OnStepChanged() override;
+ void OnChildDialogChanged() override;
+
+ raw_ptr<IsolatedWebAppInstallerModel> model_;
+ base::OnceClosure step_changed_callback_;
+ base::OnceClosure dialog_changed_callback_;
+};
+
+} // namespace web_app
+
+#endif // CHROME_BROWSER_UI_VIEWS_WEB_APPS_ISOLATED_WEB_APPS_TEST_ISOLATED_WEB_APP_INSTALLER_MODEL_OBSERVER_H_
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index c3f4b2a..cc5cdde 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -9932,6 +9932,7 @@
if (toolkit_views) {
deps += [
+ "//chrome/browser/ui:test_support",
"//chrome/browser/ui/actions:actions",
"//chrome/browser/ui/actions:actions_headers",
"//chrome/browser/ui/tabs:tab_enums",
