Eliminate all UI thread decoding of extension images....

chrome/browser/extensions/extension_install_ui.cc

Index: chrome/browser/extensions/extension_install_ui.cc
===================================================================
--- chrome/browser/extensions/extension_install_ui.cc	(revision 42362)
+++ chrome/browser/extensions/extension_install_ui.cc	(working copy)
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -127,16 +127,21 @@
 }  // namespace
 
 ExtensionInstallUI::ExtensionInstallUI(Profile* profile)
-    : profile_(profile), ui_loop_(MessageLoop::current())
+    : profile_(profile),
+      ui_loop_(MessageLoop::current()),
+      extension_(NULL),
+      delegate_(NULL),
+      ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this))
 #if defined(TOOLKIT_GTK)
-    ,previous_use_gtk_theme_(false)
+    , previous_use_gtk_theme_(false)
 #endif
 {}
 
 void ExtensionInstallUI::ConfirmInstall(Delegate* delegate,
-                                        Extension* extension,
-                                        SkBitmap* install_icon) {
+                                        Extension* extension) {
   DCHECK(ui_loop_ == MessageLoop::current());
+  extension_ = extension;
+  delegate_ = delegate;
 
   // We special-case themes to not show any confirm UI. Instead they are
   // immediately installed, and then we show an infobar (see OnInstallSuccess)
@@ -148,7 +153,7 @@
       previous_theme_id_ = previous_theme->id();
 
 #if defined(TOOLKIT_GTK)
-    // On linux, we also need to take the user's system settings into account
+    // On Linux, we also need to take the user's system settings into account
     // to undo theme installation.
     previous_use_gtk_theme_ =
         GtkThemeProvider::GetFrom(profile_)->UseGtkTheme();
@@ -158,53 +163,45 @@
     return;
   }
 
-  if (!install_icon) {
-    install_icon = ResourceBundle::GetSharedInstance().GetBitmapNamed(
-        IDR_EXTENSION_DEFAULT_ICON);
-  }
-  icon_ = *install_icon;
-
-  NotificationService* service = NotificationService::current();
-  service->Notify(NotificationType::EXTENSION_WILL_SHOW_CONFIRM_DIALOG,
-                  Source<ExtensionInstallUI>(this),
-                  NotificationService::NoDetails());
-
-  ShowExtensionInstallUIPromptImpl(
-      profile_, delegate, extension, &icon_,
-      WideToUTF16Hack(GetInstallWarning(extension)), INSTALL_PROMPT);
+  // Load the image asynchronously. For the response, check OnImageLoaded.
+  ExtensionResource image =
+      extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE);
+  tracker_.PostLoadImageTask(
+      image,
+      gfx::Size(Extension::EXTENSION_ICON_LARGE,
+                Extension::EXTENSION_ICON_LARGE),
+      IDS_EXTENSION_INSTALL_PROMPT_TITLE);
 }
 
 void ExtensionInstallUI::ConfirmUninstall(Delegate* delegate,
-                                          Extension* extension,
-                                          SkBitmap* icon) {
+                                          Extension* extension) {
   DCHECK(ui_loop_ == MessageLoop::current());
+  extension_ = extension;
+  delegate_ = delegate;
 
-  if (!icon) {
-    icon = ResourceBundle::GetSharedInstance().GetBitmapNamed(
-        IDR_EXTENSION_DEFAULT_ICON);
-  }
-
-  string16 message =
-      l10n_util::GetStringUTF16(IDS_EXTENSION_UNINSTALL_CONFIRMATION);
-  ShowExtensionInstallUIPromptImpl(profile_, delegate, extension, icon,
-                                   message, UNINSTALL_PROMPT);
+  // Load the image asynchronously. For the response, check OnImageLoaded.
+  ExtensionResource image =
+      extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE);

[Aaron Boodman, 2010/03/23 19:10:55]

Looks like you could share this code in a LoadImage() helper.

+  tracker_.PostLoadImageTask(
+      image,
+      gfx::Size(Extension::EXTENSION_ICON_LARGE,
+                Extension::EXTENSION_ICON_LARGE),
+      IDS_EXTENSION_UNINSTALL_PROMPT_TITLE);
 }
 
 void ExtensionInstallUI::ConfirmEnableIncognito(Delegate* delegate,
-                                                Extension* extension,
-                                                SkBitmap* icon) {
+                                                Extension* extension) {
   DCHECK(ui_loop_ == MessageLoop::current());
+  extension_ = extension;
+  delegate_ = delegate;
 
-  if (!icon) {
-    icon = ResourceBundle::GetSharedInstance().GetBitmapNamed(
-        IDR_EXTENSION_DEFAULT_ICON);
-  }
-
-  string16 message =
-      l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT_WARNING_INCOGNITO,
-                                 l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
-  ShowExtensionInstallUIPromptImpl(profile_, delegate, extension, icon,
-                                   message, ENABLE_INCOGNITO_PROMPT);
+  // Load the image asynchronously. For the response, check OnImageLoaded.
+  ExtensionResource image =
+      extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE);
+  tracker_.PostLoadImageTask(image,
+                             gfx::Size(Extension::EXTENSION_ICON_LARGE,
+                                       Extension::EXTENSION_ICON_LARGE),
+                             IDS_EXTENSION_ENABLE_INCOGNITO_PROMPT_TITLE);
 }
 
 void ExtensionInstallUI::OnInstallSuccess(Extension* extension) {
@@ -213,7 +210,7 @@
     return;
   }
 
-  // GetLastActiveWithProfile will fail on the build bots. This needs to
+  // GetLastActiveWithProfile will fail on the build bots. This needs to be
   // implemented differently if any test is created which depends on
   // ExtensionInstalledBubble showing.
 #if defined(TOOLKIT_VIEWS)
@@ -258,6 +255,41 @@
   ShowThemeInfoBar(extension);
 }
 
+void ExtensionInstallUI::OnImageLoaded(SkBitmap* image, int index) {
+  if (image)
+    icon_ = *image;
+  else
+    icon_ = SkBitmap();
+  if (icon_.empty()) {
+    icon_ = *ResourceBundle::GetSharedInstance().GetBitmapNamed(
+        IDR_EXTENSION_DEFAULT_ICON);
+  }
+
+  if (index == IDS_EXTENSION_INSTALL_PROMPT_TITLE) {

[Aaron Boodman, 2010/03/23 19:10:55]

whoa there cowboy. Overloading a param named "index" this way to be an opaque
identifer is ... a bit unexpected.

I think it would be simpler to store the type of dialog as a member of this
class in each of the Confirm*() methods, and then check that here.

+    NotificationService* service = NotificationService::current();
+    service->Notify(NotificationType::EXTENSION_WILL_SHOW_CONFIRM_DIALOG,
+        Source<ExtensionInstallUI>(this),
+        NotificationService::NoDetails());
+
+    ShowExtensionInstallUIPromptImpl(
+        profile_, delegate_, extension_, &icon_,
+        WideToUTF16Hack(GetInstallWarning(extension_)), INSTALL_PROMPT);
+  } else if (index == IDS_EXTENSION_UNINSTALL_PROMPT_TITLE) {
+    string16 message =
+        l10n_util::GetStringUTF16(IDS_EXTENSION_UNINSTALL_CONFIRMATION);
+    ShowExtensionInstallUIPromptImpl(profile_, delegate_, extension_, &icon_,
+        message, UNINSTALL_PROMPT);
+  } else if (index == IDS_EXTENSION_ENABLE_INCOGNITO_PROMPT_TITLE) {
+    string16 message =
+        l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT_WARNING_INCOGNITO,
+        l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
+    ShowExtensionInstallUIPromptImpl(profile_, delegate_, extension_, &icon_,
+        message, ENABLE_INCOGNITO_PROMPT);
+  } else {
+    NOTREACHED() << "Unknown message";
+  }
+}
+
 void ExtensionInstallUI::ShowThemeInfoBar(Extension* new_theme) {
   if (!new_theme->IsTheme())
     return;