Skip to content

Commit

Permalink
[Extensions refactor] Use DialogModel in SettingsOverriddenDialog
Browse files Browse the repository at this point in the history
No functionality / ui changed.

Bug: 1330637
Change-Id: I707deb0df7a48223bd5ea16fecbefd9f142cd95e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3812161
Reviewed-by: Caroline Rising <corising@chromium.org>
Commit-Queue: Emilia Paz <emiliapaz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1033174}
  • Loading branch information
emilia-paz authored and Chromium LUCI CQ committed Aug 9, 2022
1 parent bc666ca commit 744dbf4
Show file tree
Hide file tree
Showing 11 changed files with 283 additions and 350 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/ui/BUILD.gn
Expand Up @@ -5468,8 +5468,8 @@ static_library("ui") {
"views/extensions/media_galleries_dialog_views.h",
"views/extensions/media_gallery_checkbox_view.cc",
"views/extensions/media_gallery_checkbox_view.h",
"views/extensions/settings_overridden_dialog_view.cc",
"views/extensions/settings_overridden_dialog_view.h",
"views/extensions/settings_overridden_dialog.cc",
"views/extensions/settings_overridden_dialog.h",
"views/javascript_app_modal_event_blocker.h",
"web_applications/app_browser_controller.cc",
"web_applications/app_browser_controller.h",
Expand Down
11 changes: 6 additions & 5 deletions chrome/browser/ui/extensions/extensions_dialogs.h
Expand Up @@ -54,11 +54,6 @@ void ShowExtensionInstallFrictionDialog(
content::WebContents* contents,
base::OnceCallback<void(bool)> callback);

// Shows a dialog indicating that an extension has overridden a setting.
void ShowExtensionSettingsOverriddenDialog(
std::unique_ptr<SettingsOverriddenDialogController> controller,
Browser* browser);

// Shows a dialog when extensions require a refresh for their action
// to be run or blocked. The dialog content is based on whether caller
// `is_updating_permissions`. When the dialog is accepted, `callback` is
Expand All @@ -69,6 +64,12 @@ void ShowReloadPageDialog(
bool is_updating_permissions,
base::OnceClosure callback);

// Shows a dialog with a warning to the user that their settings have been
// overridden by an extension.
void ShowSettingsOverriddenDialog(
std::unique_ptr<SettingsOverriddenDialogController> controller,
Browser* browser);

#if BUILDFLAG(ENABLE_SUPERVISED_USERS)

// The type of action that the ExtensionInstalledBlockedByParentDialog
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/extensions/settings_api_bubble_helpers.cc
Expand Up @@ -153,7 +153,7 @@ void MaybeShowExtensionControlledSearchNotification(
if (!dialog->ShouldShow())
return;

ShowExtensionSettingsOverriddenDialog(std::move(dialog), browser);
ShowSettingsOverriddenDialog(std::move(dialog), browser);
#endif
}

Expand Down Expand Up @@ -201,7 +201,7 @@ void MaybeShowExtensionControlledNewTabPage(
if (!dialog->ShouldShow())
return;

ShowExtensionSettingsOverriddenDialog(std::move(dialog), browser);
ShowSettingsOverriddenDialog(std::move(dialog), browser);
}

} // namespace extensions
115 changes: 115 additions & 0 deletions chrome/browser/ui/views/extensions/settings_overridden_dialog.cc
@@ -0,0 +1,115 @@
// Copyright 2022 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/views/extensions/settings_overridden_dialog.h"

#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/extensions/settings_overridden_dialog_controller.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/grit/generated_resources.h"
#include "components/constrained_window/constrained_window_views.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/models/dialog_model.h"
#include "ui/base/models/image_model.h"
#include "ui/color/color_id.h"
#include "ui/gfx/paint_vector_icon.h"

using DialogResult = SettingsOverriddenDialogController::DialogResult;

namespace {

// Model delegate that notifies the `controller_` when a click event occurs in
// the settings overriden dialog.
class SettingsOverriddenDialogDelegate : public ui::DialogModelDelegate {
public:
explicit SettingsOverriddenDialogDelegate(
std::unique_ptr<SettingsOverriddenDialogController> controller)
: controller_(std::move(controller)) {}

void OnDialogAccepted() {
HandleDialogResult(DialogResult::kChangeSettingsBack);
}
void OnDialogCancelled() {
HandleDialogResult(DialogResult::kKeepNewSettings);
}
void OnDialogClosed() { HandleDialogResult(DialogResult::kDialogDismissed); }
void OnDialogDestroyed() {
if (!result_) {
// The dialog may close without firing any of the [accept | cancel |
// close] callbacks if e.g. the parent window closes. In this case, notify
// the controller that the dialog closed without user action.
HandleDialogResult(DialogResult::kDialogClosedWithoutUserAction);
}
}

SettingsOverriddenDialogController* controller() { return controller_.get(); }

private:
void HandleDialogResult(DialogResult result) {
DCHECK(!result_)
<< "Trying to re-notify controller of result. Previous result: "
<< static_cast<int>(*result_)
<< ", new result: " << static_cast<int>(result);
result_ = result;
controller_->HandleDialogResult(result);
}
std::unique_ptr<SettingsOverriddenDialogController> controller_;
absl::optional<DialogResult> result_;
};

} // namespace

namespace extensions {

void ShowSettingsOverriddenDialog(
std::unique_ptr<SettingsOverriddenDialogController> controller,
Browser* browser) {
SettingsOverriddenDialogController::ShowParams show_params =
controller->GetShowParams();

auto dialog_delegate_unique =
std::make_unique<SettingsOverriddenDialogDelegate>(std::move(controller));
SettingsOverriddenDialogDelegate* dialog_delegate =
dialog_delegate_unique.get();

ui::DialogModel::Builder dialog_builder =
ui::DialogModel::Builder(std::move(dialog_delegate_unique));
dialog_builder.SetInternalName(kExtensionSettingsOverridenDialogName)
.SetTitle(show_params.dialog_title)
.AddBodyText(ui::DialogModelLabel(show_params.message))
.AddOkButton(
base::BindOnce(&SettingsOverriddenDialogDelegate::OnDialogAccepted,
base::Unretained(dialog_delegate)),
l10n_util::GetStringUTF16(
IDS_EXTENSION_SETTINGS_OVERRIDDEN_DIALOG_CHANGE_IT_BACK))
.AddCancelButton(
base::BindOnce(&SettingsOverriddenDialogDelegate::OnDialogCancelled,
base::Unretained(dialog_delegate)),
l10n_util::GetStringUTF16(
IDS_EXTENSION_SETTINGS_OVERRIDDEN_DIALOG_KEEP_IT))
.SetCloseActionCallback(
base::BindOnce(&SettingsOverriddenDialogDelegate::OnDialogClosed,
base::Unretained(dialog_delegate)))
.SetDialogDestroyingCallback(
base::BindOnce(&SettingsOverriddenDialogDelegate::OnDialogDestroyed,
base::Unretained(dialog_delegate)))
.OverrideShowCloseButton(false);

if (show_params.icon) {
gfx::ImageSkia icon =
gfx::CreateVectorIcon(*show_params.icon,
ChromeLayoutProvider::Get()->GetDistanceMetric(
DISTANCE_BUBBLE_HEADER_VECTOR_ICON_SIZE),
ui::kColorIcon);

dialog_builder.SetIcon(ui::ImageModel::FromImageSkia(icon));
}

constrained_window::ShowBrowserModal(dialog_builder.Build(),
browser->window()->GetNativeWindow());
dialog_delegate->controller()->OnDialogShown();
}

} // namespace extensions
11 changes: 11 additions & 0 deletions chrome/browser/ui/views/extensions/settings_overridden_dialog.h
@@ -0,0 +1,11 @@
// Copyright 2022 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.

#ifndef CHROME_BROWSER_UI_VIEWS_EXTENSIONS_SETTINGS_OVERRIDDEN_DIALOG_H_
#define CHROME_BROWSER_UI_VIEWS_EXTENSIONS_SETTINGS_OVERRIDDEN_DIALOG_H_

static constexpr char kExtensionSettingsOverridenDialogName[] =
"ExtensionSettingsOverridenDialog";

#endif // CHROME_BROWSER_UI_VIEWS_EXTENSIONS_SETTINGS_OVERRIDDEN_DIALOG_H_
Expand Up @@ -2,19 +2,17 @@
// 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/extensions/settings_overridden_dialog_view.h"
#include "chrome/browser/ui/views/extensions/settings_overridden_dialog.h"

#include "base/memory/raw_ptr.h"
#include "base/path_service.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/extensions/chrome_test_extension_loader.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/extensions/extensions_dialogs.h"
#include "chrome/browser/ui/extensions/settings_api_bubble_helpers.h"
#include "chrome/browser/ui/extensions/settings_overridden_dialog_controller.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
Expand All @@ -29,6 +27,10 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "ui/views/test/widget_test.h"
#include "ui/views/widget/any_widget_observer.h"
#include "ui/views/widget/widget.h"

using DialogResult = SettingsOverriddenDialogController::DialogResult;

namespace {

Expand Down Expand Up @@ -100,22 +102,23 @@ class SettingsOverriddenDialogViewBrowserTest : public DialogBrowserTest {
}
}

// Creates, shows, and returns a dialog anchored to the given |browser|. The
// Creates, shows, and returns a dialog anchored to the given `browser`. The
// dialog is owned by the views framework.
SettingsOverriddenDialogView* ShowSimpleDialog(bool show_icon,
Browser* browser) {
views::Widget* ShowSimpleDialog(bool show_icon, Browser* browser) {
SettingsOverriddenDialogController::ShowParams params{
u"Settings overridden dialog title",
u"Settings overriden dialog body, which is quite a bit "
u"longer than the title alone"};
if (show_icon)
params.icon = &kProductIcon;
auto* dialog =
new SettingsOverriddenDialogView(std::make_unique<TestDialogController>(
std::move(params), &dialog_result_));
dialog->Show(browser->window()->GetNativeWindow());

return dialog;
views::NamedWidgetShownWaiter waiter(views::test::AnyWidgetTestPasskey{},
kExtensionSettingsOverridenDialogName);
extensions::ShowSettingsOverriddenDialog(
std::make_unique<TestDialogController>(std::move(params),
&dialog_result_),
browser);
return waiter.WaitIfNeededAndGet();
}

void ShowNtpOverriddenDefaultDialog() {
Expand Down Expand Up @@ -156,10 +159,7 @@ class SettingsOverriddenDialogViewBrowserTest : public DialogBrowserTest {
return true;
}

absl::optional<SettingsOverriddenDialogController::DialogResult>
dialog_result() const {
return dialog_result_;
}
absl::optional<DialogResult> dialog_result() const { return dialog_result_; }

private:
void LoadExtensionOverridingNewTab() {
Expand Down Expand Up @@ -231,9 +231,7 @@ class SettingsOverriddenDialogViewBrowserTest : public DialogBrowserTest {
}

std::string test_name_;

absl::optional<SettingsOverriddenDialogController::DialogResult>
dialog_result_;
absl::optional<DialogResult> dialog_result_;
};

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -296,15 +294,11 @@ IN_PROC_BROWSER_TEST_F(SettingsOverriddenDialogViewBrowserTest,
Browser* second_browser = CreateBrowser(browser()->profile());
ASSERT_TRUE(second_browser);

SettingsOverriddenDialogView* dialog =
ShowSimpleDialog(false, second_browser);

views::test::WidgetDestroyedWaiter widget_destroyed_waiter(
dialog->GetWidget());
views::Widget* dialog = ShowSimpleDialog(false, second_browser);
views::test::WidgetDestroyedWaiter widget_destroyed_waiter(dialog);
CloseBrowserSynchronously(second_browser);
widget_destroyed_waiter.Wait();

ASSERT_TRUE(dialog_result());
EXPECT_EQ(SettingsOverriddenDialogController::DialogResult::
kDialogClosedWithoutUserAction,
*dialog_result());
EXPECT_EQ(DialogResult::kDialogClosedWithoutUserAction, *dialog_result());
}

0 comments on commit 744dbf4

Please sign in to comment.