Refactoring ExtenionInstallUI to abstract the Browser references.

chrome/browser/extensions/extension_install_prompt.cc

Index: chrome/browser/extensions/extension_install_prompt.cc
diff --git a/chrome/browser/extensions/extension_install_ui.cc b/chrome/browser/extensions/extension_install_prompt.cc
similarity index 54%
copy from chrome/browser/extensions/extension_install_ui.cc
copy to chrome/browser/extensions/extension_install_prompt.cc
index ad858d3eddf76b20c23a1bc3b63abc24acc3bc4f..d0867026822e913e9d8f5e7be6525615640681b7 100644
--- a/chrome/browser/extensions/extension_install_ui.cc
+++ b/chrome/browser/extensions/extension_install_prompt.cc
@@ -2,7 +2,7 @@
 // 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 "chrome/browser/extensions/extension_install_prompt.h"
 
 #include <map>
 
@@ -15,29 +15,16 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/bundle_installer.h"
 #include "chrome/browser/extensions/extension_install_dialog.h"
-#include "chrome/browser/extensions/theme_installed_infobar_delegate.h"
-#include "chrome/browser/infobars/infobar_tab_helper.h"
+#include "chrome/browser/extensions/extension_install_ui.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/themes/theme_service.h"
-#include "chrome/browser/themes/theme_service_factory.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_dialogs.h"
-#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_navigator.h"
-#include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/simple_message_box.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
-#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_icon_set.h"
 #include "chrome/common/extensions/extension_manifest_constants.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/extensions/url_pattern.h"
-#include "chrome/common/url_constants.h"
-#include "content/public/browser/notification_service.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
@@ -49,39 +36,39 @@
 #include "ash/shell.h"
 #endif
 
-using content::WebContents;
 using extensions::BundleInstaller;
 using extensions::Extension;
 
-static const int kTitleIds[ExtensionInstallUI::NUM_PROMPT_TYPES] = {
+static const int kTitleIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
   0,  // The regular install prompt depends on what's being installed.
   IDS_EXTENSION_INLINE_INSTALL_PROMPT_TITLE,
   IDS_EXTENSION_INSTALL_PROMPT_TITLE,
   IDS_EXTENSION_RE_ENABLE_PROMPT_TITLE,
   IDS_EXTENSION_PERMISSIONS_PROMPT_TITLE
 };
-static const int kHeadingIds[ExtensionInstallUI::NUM_PROMPT_TYPES] = {
+static const int kHeadingIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
   IDS_EXTENSION_INSTALL_PROMPT_HEADING,
   0,  // Inline installs use the extension name.
   0,  // Heading for bundle installs depends on the bundle contents.
   IDS_EXTENSION_RE_ENABLE_PROMPT_HEADING,
   IDS_EXTENSION_PERMISSIONS_PROMPT_HEADING
 };
-static const int kAcceptButtonIds[ExtensionInstallUI::NUM_PROMPT_TYPES] = {
+static const int kAcceptButtonIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
   IDS_EXTENSION_PROMPT_INSTALL_BUTTON,
   IDS_EXTENSION_PROMPT_INSTALL_BUTTON,
   IDS_EXTENSION_PROMPT_INSTALL_BUTTON,
   IDS_EXTENSION_PROMPT_RE_ENABLE_BUTTON,
   IDS_EXTENSION_PROMPT_PERMISSIONS_BUTTON
 };
-static const int kAbortButtonIds[ExtensionInstallUI::NUM_PROMPT_TYPES] = {
+static const int kAbortButtonIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
   0,  // These all use the platform's default cancel label.
   0,
   0,
   0,
   IDS_EXTENSION_PROMPT_PERMISSIONS_ABORT_BUTTON
 };
-static const int kPermissionsHeaderIds[ExtensionInstallUI::NUM_PROMPT_TYPES] = {
+static const int kPermissionsHeaderIds[
+    ExtensionInstallPrompt::NUM_PROMPT_TYPES] = {
   IDS_EXTENSION_PROMPT_WILL_HAVE_ACCESS_TO,
   IDS_EXTENSION_PROMPT_WILL_HAVE_ACCESS_TO,
   IDS_EXTENSION_PROMPT_THESE_WILL_HAVE_ACCESS_TO,
@@ -96,7 +83,7 @@ const int kIconSize = 69;
 
 }  // namespace
 
-ExtensionInstallUI::Prompt::Prompt(PromptType type)
+ExtensionInstallPrompt::Prompt::Prompt(PromptType type)

[Yoyo Zhou, 2012/05/23 23:17:49]

Names like this make me think the structure of the code is not ideal. But such
refactoring is probably beyond the scope of this CL.

[Jay Civelli, 2012/05/30 20:20:35]

Agreed.

     : type_(type),
       extension_(NULL),
       bundle_(NULL),
@@ -104,15 +91,15 @@ ExtensionInstallUI::Prompt::Prompt(PromptType type)
       rating_count_(0) {
 }
 
-ExtensionInstallUI::Prompt::~Prompt() {
+ExtensionInstallPrompt::Prompt::~Prompt() {
 }
 
-void ExtensionInstallUI::Prompt::SetPermissions(
+void ExtensionInstallPrompt::Prompt::SetPermissions(
     const std::vector<string16>& permissions) {
   permissions_ = permissions;
 }
 
-void ExtensionInstallUI::Prompt::SetInlineInstallWebstoreData(
+void ExtensionInstallPrompt::Prompt::SetInlineInstallWebstoreData(
     const std::string& localized_user_count,
     double average_rating,
     int rating_count) {
@@ -122,7 +109,7 @@ void ExtensionInstallUI::Prompt::SetInlineInstallWebstoreData(
   rating_count_ = rating_count;
 }
 
-string16 ExtensionInstallUI::Prompt::GetDialogTitle() const {
+string16 ExtensionInstallPrompt::Prompt::GetDialogTitle() const {
   if (type_ == INSTALL_PROMPT) {
     return l10n_util::GetStringUTF16(extension_->is_app() ?
         IDS_EXTENSION_INSTALL_APP_PROMPT_TITLE :
@@ -132,7 +119,7 @@ string16 ExtensionInstallUI::Prompt::GetDialogTitle() const {
   }
 }
 
-string16 ExtensionInstallUI::Prompt::GetHeading() const {
+string16 ExtensionInstallPrompt::Prompt::GetHeading() const {
   if (type_ == INLINE_INSTALL_PROMPT) {
     return UTF8ToUTF16(extension_->name());
   } else if (type_ == BUNDLE_INSTALL_PROMPT) {
@@ -143,24 +130,24 @@ string16 ExtensionInstallUI::Prompt::GetHeading() const {
   }
 }
 
-string16 ExtensionInstallUI::Prompt::GetAcceptButtonLabel() const {
+string16 ExtensionInstallPrompt::Prompt::GetAcceptButtonLabel() const {
   return l10n_util::GetStringUTF16(kAcceptButtonIds[type_]);
 }
 
-bool ExtensionInstallUI::Prompt::HasAbortButtonLabel() const {
+bool ExtensionInstallPrompt::Prompt::HasAbortButtonLabel() const {
   return kAbortButtonIds[type_] > 0;
 }
 
-string16 ExtensionInstallUI::Prompt::GetAbortButtonLabel() const {
+string16 ExtensionInstallPrompt::Prompt::GetAbortButtonLabel() const {
   CHECK(HasAbortButtonLabel());
   return l10n_util::GetStringUTF16(kAbortButtonIds[type_]);
 }
 
-string16 ExtensionInstallUI::Prompt::GetPermissionsHeading() const {
+string16 ExtensionInstallPrompt::Prompt::GetPermissionsHeading() const {
   return l10n_util::GetStringUTF16(kPermissionsHeaderIds[type_]);
 }
 
-void ExtensionInstallUI::Prompt::AppendRatingStars(
+void ExtensionInstallPrompt::Prompt::AppendRatingStars(
     StarAppender appender, void* data) const {
   CHECK(appender);
   CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
@@ -189,32 +176,33 @@ void ExtensionInstallUI::Prompt::AppendRatingStars(
   }
 }
 
-string16 ExtensionInstallUI::Prompt::GetRatingCount() const {
+string16 ExtensionInstallPrompt::Prompt::GetRatingCount() const {
   CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
   return l10n_util::GetStringFUTF16(
         IDS_EXTENSION_RATING_COUNT,
         UTF8ToUTF16(base::IntToString(rating_count_)));
 }
 
-string16 ExtensionInstallUI::Prompt::GetUserCount() const {
+string16 ExtensionInstallPrompt::Prompt::GetUserCount() const {
   CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
   return l10n_util::GetStringFUTF16(
       IDS_EXTENSION_USER_COUNT,
       UTF8ToUTF16(localized_user_count_));
 }
 
-size_t ExtensionInstallUI::Prompt::GetPermissionCount() const {
+size_t ExtensionInstallPrompt::Prompt::GetPermissionCount() const {
   return permissions_.size();
 }
 
-string16 ExtensionInstallUI::Prompt::GetPermission(size_t index) const {
+string16 ExtensionInstallPrompt::Prompt::GetPermission(size_t index) const {
   CHECK_LT(index, permissions_.size());
   return l10n_util::GetStringFUTF16(
       IDS_EXTENSION_PERMISSION_LINE, permissions_[index]);
 }
 
 // static
-scoped_refptr<Extension> ExtensionInstallUI::GetLocalizedExtensionForDisplay(
+scoped_refptr<Extension>
+    ExtensionInstallPrompt::GetLocalizedExtensionForDisplay(
     const DictionaryValue* manifest,
     const std::string& id,
     const std::string& localized_name,
@@ -242,32 +230,23 @@ scoped_refptr<Extension> ExtensionInstallUI::GetLocalizedExtensionForDisplay(
       error);
 }
 
-ExtensionInstallUI::ExtensionInstallUI(Profile* profile)
+ExtensionInstallPrompt::ExtensionInstallPrompt(Profile* profile)
     : profile_(profile),
       ui_loop_(MessageLoop::current()),
-      previous_using_native_theme_(false),
       extension_(NULL),
+      install_ui_(ExtensionInstallUI::Create(profile)),
       delegate_(NULL),
       prompt_(UNSET_PROMPT_TYPE),
       prompt_type_(UNSET_PROMPT_TYPE),
       ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)),
       use_app_installed_bubble_(false),
       skip_post_install_ui_(false) {
-  // Remember the current theme in case the user presses undo.
-  if (profile_) {
-    const Extension* previous_theme =
-        ThemeServiceFactory::GetThemeForProfile(profile_);
-    if (previous_theme)
-      previous_theme_id_ = previous_theme->id();
-    previous_using_native_theme_ =
-        ThemeServiceFactory::GetForProfile(profile_)->UsingNativeTheme();
-  }
 }
 
-ExtensionInstallUI::~ExtensionInstallUI() {
+ExtensionInstallPrompt::~ExtensionInstallPrompt() {
 }
 
-void ExtensionInstallUI::ConfirmBundleInstall(
+void ExtensionInstallPrompt::ConfirmBundleInstall(
     extensions::BundleInstaller* bundle,
     const ExtensionPermissionSet* permissions) {
   DCHECK(ui_loop_ == MessageLoop::current());
@@ -279,11 +258,11 @@ void ExtensionInstallUI::ConfirmBundleInstall(
   ShowConfirmation();
 }
 
-void ExtensionInstallUI::ConfirmInlineInstall(
+void ExtensionInstallPrompt::ConfirmInlineInstall(
     Delegate* delegate,
     const Extension* extension,
     SkBitmap* icon,
-    const ExtensionInstallUI::Prompt& prompt) {
+    const ExtensionInstallPrompt::Prompt& prompt) {
   DCHECK(ui_loop_ == MessageLoop::current());
   extension_ = extension;
   permissions_ = extension->GetActivePermissions();
@@ -295,9 +274,9 @@ void ExtensionInstallUI::ConfirmInlineInstall(
   ShowConfirmation();
 }
 
-void ExtensionInstallUI::ConfirmWebstoreInstall(Delegate* delegate,
-                                                const Extension* extension,
-                                                const SkBitmap* icon) {
+void ExtensionInstallPrompt::ConfirmWebstoreInstall(Delegate* delegate,
+                                                    const Extension* extension,
+                                                    const SkBitmap* icon) {
   // SetIcon requires |extension_| to be set. ConfirmInstall will setup the
   // remaining fields.
   extension_ = extension;
@@ -305,8 +284,8 @@ void ExtensionInstallUI::ConfirmWebstoreInstall(Delegate* delegate,
   ConfirmInstall(delegate, extension);
 }
 
-void ExtensionInstallUI::ConfirmInstall(Delegate* delegate,
-                                        const Extension* extension) {
+void ExtensionInstallPrompt::ConfirmInstall(Delegate* delegate,
+                                            const Extension* extension) {
   DCHECK(ui_loop_ == MessageLoop::current());
   extension_ = extension;
   permissions_ = extension->GetActivePermissions();
@@ -324,8 +303,8 @@ void ExtensionInstallUI::ConfirmInstall(Delegate* delegate,
   LoadImageIfNeeded();
 }
 
-void ExtensionInstallUI::ConfirmReEnable(Delegate* delegate,
-                                         const Extension* extension) {
+void ExtensionInstallPrompt::ConfirmReEnable(Delegate* delegate,
+                                             const Extension* extension) {
   DCHECK(ui_loop_ == MessageLoop::current());
   extension_ = extension;
   permissions_ = extension->GetActivePermissions();
@@ -335,7 +314,7 @@ void ExtensionInstallUI::ConfirmReEnable(Delegate* delegate,
   LoadImageIfNeeded();
 }
 
-void ExtensionInstallUI::ConfirmPermissions(
+void ExtensionInstallPrompt::ConfirmPermissions(
     Delegate* delegate,
     const Extension* extension,
     const ExtensionPermissionSet* permissions) {
@@ -348,42 +327,15 @@ void ExtensionInstallUI::ConfirmPermissions(
   LoadImageIfNeeded();
 }
 
-void ExtensionInstallUI::OnInstallSuccess(const Extension* extension,
-                                          SkBitmap* icon) {
+void ExtensionInstallPrompt::OnInstallSuccess(const Extension* extension,
+                                              SkBitmap* icon) {
   if (skip_post_install_ui_)
     return;
 
   extension_ = extension;
   SetIcon(icon);

[Yoyo Zhou, 2012/05/23 23:17:49]

It seems surprising to me that these are necessary.
http://codereview.chromium.org/5690004 is where they were added. Maybe aa can
comment on whether they are still useful.

[Aaron Boodman, 2012/05/30 00:27:24]

No idea... this code has changed so much. It's easy enough to test though.
According to the old CL, the bug was that the icon in the install bubble did not
show up after install. So with this patch, when you install an extension, does
the install bubble show up correctly? (It's the thingy in the top right corner
pointing to either the icon in the toolbar or the wrench. It's supposed to have
the extension icon in it).

[Jay Civelli, 2012/05/30 20:20:35]

I just checked. I commented out the 2 lines above, installed  the GMail checker
app and the bubble did include the right icon. So I am assuming this is not
needed anymore.
Without this, I have been able to move this code directly to ExtensionInstallUI.

 
-  if (extension->is_theme()) {
-    ShowThemeInfoBar(previous_theme_id_, previous_using_native_theme_,
-                     extension, profile_);
-    return;
-  }
-
-  // Extensions aren't enabled by default in incognito so we confirm
-  // the install in a normal window.
-  Profile* profile = profile_->GetOriginalProfile();
-  Browser* browser = browser::FindOrCreateTabbedBrowser(profile);
-  if (browser->tab_count() == 0)
-    browser->AddBlankTab(true);
-  browser->window()->Show();
-
-  bool use_bubble_for_apps = false;
-
-#if defined(TOOLKIT_VIEWS)
-  CommandLine* cmdline = CommandLine::ForCurrentProcess();
-  use_bubble_for_apps = (use_app_installed_bubble_ ||
-                         cmdline->HasSwitch(switches::kAppsNewInstallBubble));
-#endif
-
-  if (extension->is_app() && !use_bubble_for_apps) {
-    ExtensionInstallUI::OpenAppInstalledUI(browser, extension->id());
-    return;
-  }
-
-  browser::ShowExtensionInstalledBubble(extension, browser, icon_, profile);
+  install_ui_->OnInstallSuccess(extension, icon);
 }
 
 namespace {
@@ -392,18 +344,15 @@ bool disable_failure_ui_for_tests = false;
 
 }  // namespace
 
-void ExtensionInstallUI::OnInstallFailure(const string16& error) {
+void ExtensionInstallPrompt::OnInstallFailure(const string16& error) {
   DCHECK(ui_loop_ == MessageLoop::current());
   if (disable_failure_ui_for_tests || skip_post_install_ui_)
     return;
 
-  Browser* browser = browser::FindLastActiveWithProfile(profile_);
-  browser::ShowMessageBox(browser ? browser->window()->GetNativeHandle() : NULL,
-      l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALL_FAILURE_TITLE), error,
-      browser::MESSAGE_BOX_TYPE_WARNING);
+  install_ui_->OnInstallFailure(error);
 }
 
-void ExtensionInstallUI::SetIcon(const SkBitmap* image) {
+void ExtensionInstallPrompt::SetIcon(const SkBitmap* image) {
   if (image)
     icon_ = *image;
   else
@@ -412,89 +361,19 @@ void ExtensionInstallUI::SetIcon(const SkBitmap* image) {
     icon_ = Extension::GetDefaultIcon(extension_->is_app());
 }
 
-void ExtensionInstallUI::OnImageLoaded(const gfx::Image& image,
-                                       const std::string& extension_id,
-                                       int index) {
+void ExtensionInstallPrompt::OnImageLoaded(const gfx::Image& image,
+                                           const std::string& extension_id,
+                                           int index) {
   SetIcon(image.IsEmpty() ? NULL : image.ToSkBitmap());
   ShowConfirmation();
 }
 
 // static
-void ExtensionInstallUI::OpenAppInstalledUI(Browser* browser,
-                                            const std::string& app_id) {
-  if (NewTabUI::ShouldShowApps()) {
-    browser::NavigateParams params = browser->GetSingletonTabNavigateParams(
-        GURL(chrome::kChromeUINewTabURL));
-    browser::Navigate(&params);
-
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_APP_INSTALLED_TO_NTP,
-        content::Source<WebContents>(params.target_contents->web_contents()),
-        content::Details<const std::string>(&app_id));
-  } else {
-#if defined(USE_ASH)
-    ash::Shell::GetInstance()->ToggleAppList();
-
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_APP_INSTALLED_TO_APPLIST,
-        content::Source<Profile>(browser->profile()),
-        content::Details<const std::string>(&app_id));
-#else
-    NOTREACHED();
-#endif
-  }
-}
-
-// static
-void ExtensionInstallUI::DisableFailureUIForTests() {
+void ExtensionInstallPrompt::DisableFailureUIForTests() {
   disable_failure_ui_for_tests = true;
 }
 
-void ExtensionInstallUI::ShowThemeInfoBar(const std::string& previous_theme_id,
-                                          bool previous_using_native_theme,
-                                          const Extension* new_theme,
-                                          Profile* profile) {
-  if (!new_theme->is_theme())
-    return;
-
-  // Get last active tabbed browser of profile.
-  Browser* browser = browser::FindTabbedBrowser(profile, true);
-  if (!browser)
-    return;
-
-  TabContentsWrapper* tab_contents = browser->GetSelectedTabContentsWrapper();
-  if (!tab_contents)
-    return;
-  InfoBarTabHelper* infobar_helper = tab_contents->infobar_tab_helper();
-
-  // First find any previous theme preview infobars.
-  InfoBarDelegate* old_delegate = NULL;
-  for (size_t i = 0; i < infobar_helper->infobar_count(); ++i) {
-    InfoBarDelegate* delegate = infobar_helper->GetInfoBarDelegateAt(i);
-    ThemeInstalledInfoBarDelegate* theme_infobar =
-        delegate->AsThemePreviewInfobarDelegate();
-    if (theme_infobar) {
-      // If the user installed the same theme twice, ignore the second install
-      // and keep the first install info bar, so that they can easily undo to
-      // get back the previous theme.
-      if (theme_infobar->MatchesTheme(new_theme))
-        return;
-      old_delegate = delegate;
-      break;
-    }
-  }
-
-  // Then either replace that old one or add a new one.
-  InfoBarDelegate* new_delegate = GetNewThemeInstalledInfoBarDelegate(
-      tab_contents, new_theme, previous_theme_id, previous_using_native_theme);
-
-  if (old_delegate)
-    infobar_helper->ReplaceInfoBar(old_delegate, new_delegate);
-  else
-    infobar_helper->AddInfoBar(new_delegate);
-}
-
-void ExtensionInstallUI::LoadImageIfNeeded() {
+void ExtensionInstallPrompt::LoadImageIfNeeded() {
   // Bundle install prompts do not have an icon.
   if (!icon_.empty()) {
     ShowConfirmation();
@@ -510,7 +389,7 @@ void ExtensionInstallUI::LoadImageIfNeeded() {
                      ImageLoadingTracker::DONT_CACHE);
 }
 
-void ExtensionInstallUI::ShowConfirmation() {
+void ExtensionInstallPrompt::ShowConfirmation() {
   prompt_.set_type(prompt_type_);
   prompt_.SetPermissions(permissions_->GetWarningMessages());
 
@@ -534,18 +413,3 @@ void ExtensionInstallUI::ShowConfirmation() {
       break;
   }
 }
-
-InfoBarDelegate* ExtensionInstallUI::GetNewThemeInstalledInfoBarDelegate(
-    TabContentsWrapper* tab_contents,
-    const Extension* new_theme,
-    const std::string& previous_theme_id,
-    bool previous_using_native_theme) {
-  Profile* profile = tab_contents->profile();
-  return new ThemeInstalledInfoBarDelegate(
-      tab_contents->infobar_tab_helper(),
-      profile->GetExtensionService(),
-      ThemeServiceFactory::GetForProfile(profile),
-      new_theme,
-      previous_theme_id,
-      previous_using_native_theme);
-}