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

chrome/browser/extensions/extension_install_ui.cc

-// 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.
 
 #include "chrome/browser/extensions/extension_install_ui.h"
 
 #include <map>
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/file_util.h"
@@ skipping 108 matching lines @@
   DCHECK(hosts.size() == 0);
   if (extension->api_permissions().empty())
     return L"";
   else
     return l10n_util::GetString(IDS_EXTENSION_PROMPT_WARNING_NEW_BROWSER);
 }
 
 }  // 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)
   // to allow the user to revert if they don't like it.
   if (extension->IsTheme()) {
     // Remember the current theme in case the user pressed undo.
     Extension* previous_theme = profile_->GetTheme();
     if (previous_theme)
       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();
 #endif
 
     delegate->InstallUIProceed(false);
     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) {
   if (extension->IsTheme()) {
     ShowThemeInfoBar(extension);
     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)
   Browser* browser = BrowserList::GetLastActiveWithProfile(profile_);
   if (!browser)
     return;
 
   ExtensionInstalledBubble::Show(extension, browser, icon_);
 #elif defined(OS_MACOSX)
   if (extension->browser_action() ||
@@ skipping 24 matching lines @@
   platform_util::SimpleErrorBox(
       browser ? browser->window()->GetNativeHandle() : NULL,
       l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALL_FAILURE_TITLE),
       UTF8ToUTF16(error));
 }
 
 void ExtensionInstallUI::OnOverinstallAttempted(Extension* extension) {
   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;
 
   Browser* browser = BrowserList::GetLastActiveWithProfile(profile_);
   if (!browser)
     return;
 
   TabContents* tab_contents = browser->GetSelectedTabContents();
   if (!tab_contents)
@@ skipping 42 matching lines @@
 InfoBarDelegate* ExtensionInstallUI::GetNewInfoBarDelegate(
     Extension* new_theme, TabContents* tab_contents) {
 #if defined(TOOLKIT_GTK)
   return new GtkThemeInstalledInfoBarDelegate(tab_contents, new_theme,
       previous_theme_id_, previous_use_gtk_theme_);
 #else
   return new ThemeInstalledInfoBarDelegate(tab_contents, new_theme,
       previous_theme_id_);
 #endif
 }