Fix uninstall extension dialog has different icon.
This CL sync icon used for extension dialog with default one used to
represent extension based app.
This additionally fixes following issues:
- base class for uninstall dialog and implementation has own
expectations for icon size. Now use only one source to define icon
size.
- Provide correct loading of icons for non-standard device scale
factors.
For mechanical change in extension_uninstall_dialog_cocoa.mm
[email protected]
desktop.
Bug: 730853
Test: Manually on Cave (1.25 scale factor), Linux Chrome OS + Linux
Change-Id: Ib3d1c80d6883a079728b917efc726e9458b6bacb
Reviewed-on: https://chromium-review.googlesource.com/527527
Commit-Queue: Yury Khmel <[email protected]>
Reviewed-by: Devlin <[email protected]>
Reviewed-by: Xiyuan Xia <[email protected]>
Cr-Commit-Position: refs/heads/master@{#481802}diff --git a/chrome/browser/extensions/extension_uninstall_dialog.cc b/chrome/browser/extensions/extension_uninstall_dialog.cc
index 5862abf..11405b7 100644
--- a/chrome/browser/extensions/extension_uninstall_dialog.cc
+++ b/chrome/browser/extensions/extension_uninstall_dialog.cc
@@ -9,11 +9,13 @@
#include "base/message_loop/message_loop.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/extensions/chrome_app_icon_service.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser_navigator.h"
#include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/browser/ui/native_window_tracker.h"
#include "chrome/grit/generated_resources.h"
#include "extensions/browser/extension_dialog_auto_confirm.h"
#include "extensions/browser/extension_registry.h"
@@ -21,14 +23,15 @@
#include "extensions/browser/image_loader.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
-#include "extensions/common/extension_icon_set.h"
-#include "extensions/common/extension_resource.h"
#include "extensions/common/extension_urls.h"
#include "extensions/common/manifest_handlers/icons_handler.h"
#include "extensions/common/manifest_url_handlers.h"
#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/layout.h"
#include "ui/base/page_transition_types.h"
#include "ui/base/window_open_disposition.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h"
@@ -36,28 +39,34 @@
namespace {
-const char kExtensionRemovedError[] =
+constexpr int kIconSize = 64;
+
+constexpr char kExtensionRemovedError[] =
"Extension was removed before dialog closed.";
-const char kReferrerId[] = "chrome-remove-extension-dialog";
+constexpr char kReferrerId[] = "chrome-remove-extension-dialog";
-// Returns gfx::ImageSkia for the default icon.
-gfx::ImageSkia GetDefaultIconImage(bool is_app) {
- return is_app ? util::GetDefaultAppIcon() : util::GetDefaultExtensionIcon();
+float GetScaleFactor(gfx::NativeWindow window) {
+ const display::Screen* screen = display::Screen::GetScreen();
+ if (!screen)
+ return 1.0; // Happens in unit_tests.
+ if (window)
+ return screen->GetDisplayNearestWindow(window).device_scale_factor();
+ return screen->GetPrimaryDisplay().device_scale_factor();
}
} // namespace
ExtensionUninstallDialog::ExtensionUninstallDialog(
Profile* profile,
+ gfx::NativeWindow parent,
ExtensionUninstallDialog::Delegate* delegate)
- : profile_(profile),
- delegate_(delegate),
- uninstall_reason_(UNINSTALL_REASON_FOR_TESTING) {
+ : profile_(profile), parent_(parent), delegate_(delegate), observer_(this) {
+ if (parent)
+ parent_window_tracker_ = NativeWindowTracker::Create(parent);
}
-ExtensionUninstallDialog::~ExtensionUninstallDialog() {
-}
+ExtensionUninstallDialog::~ExtensionUninstallDialog() = default;
void ExtensionUninstallDialog::ConfirmUninstallByExtension(
const scoped_refptr<const Extension>& extension,
@@ -79,48 +88,36 @@
extension_ = extension;
uninstall_reason_ = reason;
- // Bookmark apps may not have 128x128 icons so accept 64x64 icons.
- const int icon_size = extension_->from_bookmark()
- ? extension_misc::EXTENSION_ICON_SMALL * 2
- : extension_misc::EXTENSION_ICON_LARGE;
- ExtensionResource image = IconsInfo::GetIconResource(
- extension_.get(), icon_size, ExtensionIconSet::MATCH_BIGGER);
- // Load the image asynchronously. The response will be sent to OnImageLoaded.
- ImageLoader* loader = ImageLoader::Get(profile_);
-
- SetIcon(gfx::Image());
- std::vector<ImageLoader::ImageRepresentation> images_list;
- images_list.push_back(ImageLoader::ImageRepresentation(
- image,
- ImageLoader::ImageRepresentation::NEVER_RESIZE,
- gfx::Size(),
- ui::SCALE_FACTOR_100P));
- loader->LoadImagesAsync(extension_.get(), images_list,
- base::Bind(&ExtensionUninstallDialog::OnImageLoaded,
- AsWeakPtr(), extension_->id()));
-}
-
-void ExtensionUninstallDialog::SetIcon(const gfx::Image& image) {
- if (image.IsEmpty()) {
- icon_ = GetDefaultIconImage(extension_->is_app());
- } else {
- icon_ = *image.ToImageSkia();
- }
-}
-
-void ExtensionUninstallDialog::OnImageLoaded(const std::string& extension_id,
- const gfx::Image& image) {
- const Extension* target_extension =
- ExtensionRegistry::Get(profile_)
- ->GetExtensionById(extension_id, ExtensionRegistry::EVERYTHING);
- if (!target_extension) {
- delegate_->OnExtensionUninstallDialogClosed(
- false, base::ASCIIToUTF16(kExtensionRemovedError));
+ if (parent() && parent_window_tracker_->WasNativeWindowClosed()) {
+ OnDialogClosed(CLOSE_ACTION_CANCELED);
return;
}
- SetIcon(image);
+ // Track that extension uninstalled externally.
+ DCHECK(!observer_.IsObserving(ExtensionRegistry::Get(profile_)));
+ observer_.Add(ExtensionRegistry::Get(profile_));
+
+ // Dialog will be shown once icon is loaded.
+ DCHECK(!dialog_shown_);
+ icon_ = ChromeAppIconService::Get(profile_)->CreateIcon(this, extension->id(),
+ kIconSize);
+ icon_->image_skia().GetRepresentation(GetScaleFactor(parent_));
+}
+
+void ExtensionUninstallDialog::OnIconUpdated(ChromeAppIcon* icon) {
+ // Ignore initial update.
+ if (!icon_ || dialog_shown_)
+ return;
+
+ DCHECK_EQ(icon, icon_.get());
+
+ dialog_shown_ = true;
+
+ if (parent() && parent_window_tracker_->WasNativeWindowClosed()) {
+ OnDialogClosed(CLOSE_ACTION_CANCELED);
+ return;
+ }
switch (ScopedTestDialogAutoConfirm::GetAutoConfirmValue()) {
case ScopedTestDialogAutoConfirm::NONE:
@@ -138,6 +135,19 @@
}
}
+void ExtensionUninstallDialog::OnExtensionUninstalled(
+ content::BrowserContext* browser_context,
+ const Extension* extension,
+ UninstallReason reason) {
+ // Handle the case when extension was uninstalled externally and we have to
+ // close current dialog.
+ if (extension != extension_)
+ return;
+
+ delegate_->OnExtensionUninstallDialogClosed(
+ false, base::ASCIIToUTF16(kExtensionRemovedError));
+}
+
std::string ExtensionUninstallDialog::GetHeadingText() {
if (triggering_extension_) {
return l10n_util::GetStringFUTF8(
@@ -191,6 +201,8 @@
ExtensionRegistry::Get(profile_)->GetExtensionById(
extension_->id(), ExtensionRegistry::EVERYTHING);
if (current_extension) {
+ // Prevent notifications triggered by our request.
+ observer_.RemoveAll();
return ExtensionSystem::Get(profile_)
->extension_service()
->UninstallExtension(extension_->id(), uninstall_reason_,
