diff options
author | rdevlin.cronin <rdevlin.cronin@chromium.org> | 2015-02-23 11:43:14 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-02-23 19:43:48 +0000 |
commit | f8a6bf68e9bafb0cf77cbf132aa69a745b737726 (patch) | |
tree | ea5f5ee5761e949acce82d83c3b628f2661e6158 | |
parent | 0b384f1f3709333a2a4baf9de5661e1dbda40fa1 (diff) | |
download | chromium_src-f8a6bf68e9bafb0cf77cbf132aa69a745b737726.zip chromium_src-f8a6bf68e9bafb0cf77cbf132aa69a745b737726.tar.gz chromium_src-f8a6bf68e9bafb0cf77cbf132aa69a745b737726.tar.bz2 |
[Extensions UI] Share more code in ExtensionInstalledBubble
Move implemention for getting the how-to-use description to
the platform-agnostic class.
BUG=458301
TBR=sky@chromium.org (OWNERS for ui/extensions; Finnur reviewed)
Review URL: https://codereview.chromium.org/922523005
Cr-Commit-Position: refs/heads/master@{#317625}
5 files changed, 91 insertions, 178 deletions
diff --git a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h index 11053b4..4840414 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h +++ b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h @@ -7,8 +7,8 @@ #import <Cocoa/Cocoa.h> +#include "base/mac/scoped_nsobject.h" #include "base/memory/scoped_ptr.h" -#import "chrome/browser/ui/cocoa/browser_window_controller.h" #import "chrome/browser/ui/cocoa/base_bubble_controller.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -17,7 +17,6 @@ class ExtensionInstalledBubble; class ExtensionInstalledBubbleBridge; @class HyperlinkTextView; @class HoverCloseButton; -@class InfoBubbleView; namespace extensions { class BundleInstaller; diff --git a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm index 465614e..88133f8 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm @@ -5,16 +5,12 @@ #import "chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h" #include "base/i18n/rtl.h" -#include "base/mac/bundle_locations.h" #include "base/memory/scoped_ptr.h" #include "base/strings/sys_string_conversions.h" #include "base/strings/utf_string_conversions.h" -#include "chrome/browser/chrome_notification_types.h" -#include "chrome/browser/extensions/api/commands/command_service.h" #include "chrome/browser/extensions/bundle_installer.h" #include "chrome/browser/extensions/extension_action.h" #include "chrome/browser/extensions/extension_action_manager.h" -#include "chrome/browser/signin/signin_promo.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_navigator.h" #include "chrome/browser/ui/browser_window.h" @@ -33,16 +29,13 @@ #include "chrome/browser/ui/extensions/extension_installed_bubble.h" #include "chrome/browser/ui/singleton_tabs.h" #include "chrome/browser/ui/sync/sync_promo_ui.h" -#include "chrome/common/extensions/api/commands/commands_handler.h" #include "chrome/common/extensions/api/extension_action/action_info.h" #include "chrome/common/extensions/api/omnibox/omnibox_handler.h" #include "chrome/common/extensions/sync_helper.h" #include "chrome/common/url_constants.h" #include "chrome/grit/chromium_strings.h" #include "chrome/grit/generated_resources.h" -#include "content/public/browser/notification_details.h" -#include "content/public/browser/notification_registrar.h" -#include "content/public/browser/notification_source.h" +#include "components/signin/core/browser/signin_metrics.h" #include "extensions/browser/install/extension_install_ui.h" #include "extensions/common/extension.h" #include "extensions/common/feature_switch.h" @@ -54,7 +47,6 @@ using content::BrowserThread; using extensions::BundleInstaller; using extensions::Extension; -using extensions::UnloadedExtensionInfo; class ExtensionInstalledBubbleBridge : public ExtensionInstalledBubble::Delegate { @@ -328,65 +320,6 @@ bool ExtensionInstalledBubbleBridge::MaybeShowNow() { return window; } -- (bool)hasActivePageAction:(extensions::Command*)command { - extensions::CommandService* command_service = - extensions::CommandService::Get(browser_->profile()); - if (type_ == extension_installed_bubble::kPageAction) { - if (extensions::CommandsInfo::GetPageActionCommand([self extension]) && - command_service->GetPageActionCommand( - [self extension]->id(), - extensions::CommandService::ACTIVE, - command, - NULL)) { - return true; - } - } - - return false; -} - -- (bool)hasActiveBrowserAction:(extensions::Command*)command { - extensions::CommandService* command_service = - extensions::CommandService::Get(browser_->profile()); - if (type_ == extension_installed_bubble::kBrowserAction) { - if (extensions::CommandsInfo::GetBrowserActionCommand([self extension]) && - command_service->GetBrowserActionCommand( - [self extension]->id(), - extensions::CommandService::ACTIVE, - command, - NULL)) { - return true; - } - } - - return false; -} - -- (NSString*)installMessageForCurrentExtensionAction { - if (type_ == extension_installed_bubble::kPageAction) { - extensions::Command page_action_command; - if ([self hasActivePageAction:&page_action_command]) { - return l10n_util::GetNSStringF( - IDS_EXTENSION_INSTALLED_PAGE_ACTION_INFO_WITH_SHORTCUT, - page_action_command.accelerator().GetShortcutText()); - } else { - return l10n_util::GetNSString( - IDS_EXTENSION_INSTALLED_PAGE_ACTION_INFO); - } - } else { - CHECK_EQ(extension_installed_bubble::kBrowserAction, type_); - extensions::Command browser_action_command; - if ([self hasActiveBrowserAction:&browser_action_command]) { - return l10n_util::GetNSStringF( - IDS_EXTENSION_INSTALLED_BROWSER_ACTION_INFO_WITH_SHORTCUT, - browser_action_command.accelerator().GetShortcutText()); - } else { - return l10n_util::GetNSString( - IDS_EXTENSION_INSTALLED_BROWSER_ACTION_INFO); - } - } -} - // Calculate the height of each install message, resizing messages in their // frames to fit window width. Return the new window height, based on the // total of all message heights. @@ -459,8 +392,8 @@ bool ExtensionInstalledBubbleBridge::MaybeShowNow() { // If type is browser/page action, include a special message about them. if (type_ == extension_installed_bubble::kBrowserAction || type_ == extension_installed_bubble::kPageAction) { - [howToUse_ setStringValue:[self - installMessageForCurrentExtensionAction]]; + [howToUse_ setStringValue:base::SysUTF16ToNSString( + installedBubble_->GetHowToUseDescription())]; [howToUse_ setHidden:NO]; [[howToUse_ cell] setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]]; @@ -472,10 +405,8 @@ bool ExtensionInstalledBubbleBridge::MaybeShowNow() { // If type is omnibox keyword, include a special message about the keyword. if (type_ == extension_installed_bubble::kOmniboxKeyword) { - [howToUse_ setStringValue:l10n_util::GetNSStringF( - IDS_EXTENSION_INSTALLED_OMNIBOX_KEYWORD_INFO, - base::UTF8ToUTF16(extensions::OmniboxInfo::GetKeyword( - [self extension])))]; + [howToUse_ setStringValue:base::SysUTF16ToNSString( + installedBubble_->GetHowToUseDescription())]; [howToUse_ setHidden:NO]; [[howToUse_ cell] setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]]; @@ -510,9 +441,7 @@ bool ExtensionInstalledBubbleBridge::MaybeShowNow() { newWindowHeight += sync_promo_height; } - extensions::Command command; - if ([self hasActivePageAction:&command] || - [self hasActiveBrowserAction:&command]) { + if (installedBubble_->has_command_keybinding()) { [manageShortcutLink_ setHidden:NO]; [[manageShortcutLink_ cell] setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]]; @@ -627,7 +556,6 @@ bool ExtensionInstalledBubbleBridge::MaybeShowNow() { [promo_.get() setFrame:frame]; } - extensions::Command command; if (![manageShortcutLink_ isHidden]) { NSRect manageShortcutFrame = [manageShortcutLink_ frame]; manageShortcutFrame.origin.y = NSMinY(frame) - ( diff --git a/chrome/browser/ui/extensions/extension_installed_bubble.cc b/chrome/browser/ui/extensions/extension_installed_bubble.cc index ea6a684..9e260ea 100644 --- a/chrome/browser/ui/extensions/extension_installed_bubble.cc +++ b/chrome/browser/ui/extensions/extension_installed_bubble.cc @@ -8,18 +8,22 @@ #include "base/bind.h" #include "base/message_loop/message_loop.h" +#include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "chrome/browser/chrome_notification_types.h" +#include "chrome/browser/extensions/api/commands/command_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/extensions/api/extension_action/action_info.h" #include "chrome/common/extensions/api/omnibox/omnibox_handler.h" +#include "chrome/common/extensions/command.h" +#include "chrome/grit/generated_resources.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" #include "extensions/browser/extension_registry.h" #include "extensions/common/extension.h" +#include "ui/base/l10n/l10n_util.h" -using content::Details; using extensions::Extension; namespace { @@ -29,6 +33,28 @@ const int kAnimationWaitMs = 50; // How often we retry when waiting for browser action animation to end. const int kAnimationWaitRetries = 10; +// Returns the keybinding for an extension command, or a null if none exists. +scoped_ptr<extensions::Command> GetCommand( + const std::string& extension_id, + Profile* profile, + ExtensionInstalledBubble::BubbleType type) { + scoped_ptr<extensions::Command> result; + extensions::Command command; + extensions::CommandService* command_service = + extensions::CommandService::Get(profile); + bool has_command = false; + if (type == ExtensionInstalledBubble::BROWSER_ACTION) { + has_command = command_service->GetBrowserActionCommand( + extension_id, extensions::CommandService::ACTIVE, &command, nullptr); + } else if (type == ExtensionInstalledBubble::PAGE_ACTION) { + has_command = command_service->GetPageActionCommand( + extension_id, extensions::CommandService::ACTIVE, &command, nullptr); + } + if (has_command) + result.reset(new extensions::Command(command)); + return result.Pass(); +} + } // namespace ExtensionInstalledBubble::ExtensionInstalledBubble(Delegate* delegate, @@ -71,6 +97,36 @@ void ExtensionInstalledBubble::IgnoreBrowserClosing() { content::Source<Browser>(browser_)); } +base::string16 ExtensionInstalledBubble::GetHowToUseDescription() const { + int message_id = 0; + base::string16 extra; + if (action_command_) + extra = action_command_->accelerator().GetShortcutText(); + + switch (type_) { + case BROWSER_ACTION: + message_id = extra.empty() ? IDS_EXTENSION_INSTALLED_BROWSER_ACTION_INFO : + IDS_EXTENSION_INSTALLED_BROWSER_ACTION_INFO_WITH_SHORTCUT; + break; + case PAGE_ACTION: + message_id = extra.empty() ? IDS_EXTENSION_INSTALLED_PAGE_ACTION_INFO : + IDS_EXTENSION_INSTALLED_PAGE_ACTION_INFO_WITH_SHORTCUT; + break; + case OMNIBOX_KEYWORD: + extra = + base::UTF8ToUTF16(extensions::OmniboxInfo::GetKeyword(extension_)); + message_id = IDS_EXTENSION_INSTALLED_OMNIBOX_KEYWORD_INFO; + break; + case GENERIC: + break; + } + + if (message_id == 0) + return base::string16(); + return extra.empty() ? l10n_util::GetStringUTF16(message_id) : + l10n_util::GetStringFUTF16(message_id, extra); +} + void ExtensionInstalledBubble::ShowInternal() { if (delegate_->MaybeShowNow()) return; @@ -87,6 +143,9 @@ void ExtensionInstalledBubble::OnExtensionLoaded( content::BrowserContext* browser_context, const extensions::Extension* extension) { if (extension == extension_) { + // Parse the extension command, if one exists. + action_command_ = GetCommand(extension_->id(), browser_->profile(), type_); + animation_wait_retries_ = 0; // PostTask to ourself to allow all EXTENSION_LOADED Observers to run. base::MessageLoopForUI::current()->PostTask( diff --git a/chrome/browser/ui/extensions/extension_installed_bubble.h b/chrome/browser/ui/extensions/extension_installed_bubble.h index 08bbf95..dddf228 100644 --- a/chrome/browser/ui/extensions/extension_installed_bubble.h +++ b/chrome/browser/ui/extensions/extension_installed_bubble.h @@ -15,6 +15,7 @@ class Browser; namespace extensions { +class Command; class Extension; class ExtensionRegistry; } @@ -64,10 +65,14 @@ class ExtensionInstalledBubble : public content::NotificationObserver, const Browser* browser() const { return browser_; } const SkBitmap& icon() const { return icon_; } BubbleType type() const { return type_; } + bool has_command_keybinding() const { return action_command_; } // Stop listening to NOTIFICATION_BROWSER_CLOSING. void IgnoreBrowserClosing(); + // Returns the string describing how to use the new extension. + base::string16 GetHowToUseDescription() const; + private: // Delegates showing the view to our |view_|. Called internally via PostTask. void ShowInternal(); @@ -95,6 +100,9 @@ class ExtensionInstalledBubble : public content::NotificationObserver, BubbleType type_; content::NotificationRegistrar registrar_; + // The command to execute the extension action, if one exists. + scoped_ptr<extensions::Command> action_command_; + // Listen to extension load, unloaded notifications. ScopedObserver<extensions::ExtensionRegistry, extensions::ExtensionRegistryObserver> diff --git a/chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc b/chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc index 7e06ba9..b51ff41 100644 --- a/chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc +++ b/chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc @@ -9,8 +9,6 @@ #include "base/i18n/rtl.h" #include "base/strings/utf_string_conversions.h" -#include "chrome/browser/chrome_notification_types.h" -#include "chrome/browser/extensions/api/commands/command_service.h" #include "chrome/browser/extensions/extension_action.h" #include "chrome/browser/extensions/extension_action_manager.h" #include "chrome/browser/profiles/profile.h" @@ -28,7 +26,6 @@ #include "chrome/browser/ui/views/toolbar/toolbar_action_view.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/browser/ui/views/toolbar/wrench_toolbar_button.h" -#include "chrome/common/extensions/api/omnibox/omnibox_handler.h" #include "chrome/common/extensions/sync_helper.h" #include "chrome/common/url_constants.h" #include "chrome/grit/chromium_strings.h" @@ -89,10 +86,8 @@ class InstalledBubbleContent : public views::View, public views::ButtonListener, public views::LinkListener { public: - InstalledBubbleContent(Browser* browser, - const Extension* extension, - ExtensionInstalledBubble::BubbleType type, - const SkBitmap* icon); + InstalledBubbleContent(const ExtensionInstalledBubble& bubble, + Browser* browser); // Overridden from views::ButtonListener. void ButtonPressed(views::Button* sender, const ui::Event& event) override; @@ -109,9 +104,6 @@ class InstalledBubbleContent : public views::View, SIGN_IN_PROMO = 1 << 3, }; - bool GetKeybinding(extensions::Command* command); - base::string16 GetHowToUseDescription(const base::string16& key); - // Layout the signin promo at coordinates |offset_x| and |offset_y|. Returns // the height (in pixels) of the promo UI. int LayoutSigninPromo(int offset_x, int offset_y); @@ -124,9 +116,6 @@ class InstalledBubbleContent : public views::View, // The browser we're associated with. Browser* browser_; - // The id of the extension just installed. - const std::string extension_id_; - // The string that contains the link text at the beginning of the sign-in // promo text. base::string16 signin_promo_link_text_; @@ -138,9 +127,6 @@ class InstalledBubbleContent : public views::View, // whited out so the link can be drawn in its place. ScopedVector<gfx::RenderText> sign_in_promo_lines_; - // The type of the bubble to show (Browser Action, Omnibox keyword, etc). - ExtensionInstalledBubble::BubbleType type_; - // A bitmask containing the various flavors of bubble sections to show. int flavors_; @@ -159,13 +145,9 @@ class InstalledBubbleContent : public views::View, }; InstalledBubbleContent::InstalledBubbleContent( - Browser* browser, - const Extension* extension, - ExtensionInstalledBubble::BubbleType type, - const SkBitmap* icon) + const ExtensionInstalledBubble& bubble, + Browser* browser) : browser_(browser), - extension_id_(extension->id()), - type_(type), flavors_(NONE), height_of_signin_promo_(0u), how_to_use_(NULL), @@ -189,23 +171,18 @@ InstalledBubbleContent::InstalledBubbleContent( // or a link to configure the keybinding shortcut (if one exists). // Extra info can include a promo for signing into sync. - // First figure out the keybinding situation. - extensions::Command command; - bool has_keybinding = GetKeybinding(&command); - base::string16 key; // Keyboard shortcut or keyword to display in bubble. - + const Extension* extension = bubble.extension(); if (extensions::sync_helper::IsSyncableExtension(extension) && SyncPromoUI::ShouldShowSyncPromo(browser->profile())) flavors_ |= SIGN_IN_PROMO; // Determine the bubble flavor we want, based on the extension type. - switch (type_) { + switch (bubble.type()) { case ExtensionInstalledBubble::BROWSER_ACTION: - case ExtensionInstalledBubble::PAGE_ACTION: { + case ExtensionInstalledBubble::PAGE_ACTION: flavors_ |= HOW_TO_USE; - if (has_keybinding) { + if (bubble.has_command_keybinding()) { flavors_ |= SHOW_KEYBINDING; - key = command.accelerator().GetShortcutText(); } else { // The How-To-Use text makes the bubble seem a little crowded when the // extension has a keybinding, so the How-To-Manage text is not shown @@ -213,36 +190,32 @@ InstalledBubbleContent::InstalledBubbleContent( flavors_ |= HOW_TO_MANAGE; } break; - } - case ExtensionInstalledBubble::OMNIBOX_KEYWORD: { + case ExtensionInstalledBubble::OMNIBOX_KEYWORD: flavors_ |= HOW_TO_USE | HOW_TO_MANAGE; - key = base::UTF8ToUTF16(extensions::OmniboxInfo::GetKeyword(extension)); break; - } - case ExtensionInstalledBubble::GENERIC: { + case ExtensionInstalledBubble::GENERIC: break; - } - default: { + default: // When adding a new bubble type, the flavor needs to be set. static_assert(ExtensionInstalledBubble::GENERIC == 3, "kBubbleType enum has changed, this switch statement must " "be updateed"); break; - } } ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); const gfx::FontList& font_list = rb.GetFontList(ui::ResourceBundle::BaseFont); + const SkBitmap& icon = bubble.icon(); // Add the icon (for all flavors). // Scale down to 43x43, but allow smaller icons (don't scale up). - gfx::Size size(icon->width(), icon->height()); + gfx::Size size(icon.width(), icon.height()); if (size.width() > kIconSize || size.height() > kIconSize) size = gfx::Size(kIconSize, kIconSize); icon_ = new views::ImageView(); icon_->SetImageSize(size); - icon_->SetImage(gfx::ImageSkia::CreateFrom1xBitmap(*icon)); + icon_->SetImage(gfx::ImageSkia::CreateFrom1xBitmap(icon)); AddChildView(icon_); // Add the heading (for all flavors). @@ -256,7 +229,7 @@ InstalledBubbleContent::InstalledBubbleContent( AddChildView(heading_); if (flavors_ & HOW_TO_USE) { - how_to_use_ = new views::Label(GetHowToUseDescription(key)); + how_to_use_ = new views::Label(bubble.GetHowToUseDescription()); how_to_use_->SetFontList(font_list); how_to_use_->SetMultiLine(true); how_to_use_->SetHorizontalAlignment(gfx::ALIGN_LEFT); @@ -326,58 +299,6 @@ void InstalledBubbleContent::LinkClicked(views::Link* source, int event_flags) { chrome::Navigate(¶ms); } -bool InstalledBubbleContent::GetKeybinding(extensions::Command* command) { - extensions::CommandService* command_service = - extensions::CommandService::Get(browser_->profile()); - if (type_ == ExtensionInstalledBubble::BROWSER_ACTION) { - return command_service->GetBrowserActionCommand( - extension_id_, - extensions::CommandService::ACTIVE, - command, - NULL); - } else if (type_ == ExtensionInstalledBubble::PAGE_ACTION) { - return command_service->GetPageActionCommand( - extension_id_, - extensions::CommandService::ACTIVE, - command, - NULL); - } else { - return false; - } -} - -base::string16 InstalledBubbleContent::GetHowToUseDescription( - const base::string16& key) { - switch (type_) { - case ExtensionInstalledBubble::BROWSER_ACTION: - if (!key.empty()) { - return l10n_util::GetStringFUTF16( - IDS_EXTENSION_INSTALLED_BROWSER_ACTION_INFO_WITH_SHORTCUT, key); - } else { - return l10n_util::GetStringUTF16( - IDS_EXTENSION_INSTALLED_BROWSER_ACTION_INFO); - } - break; - case ExtensionInstalledBubble::PAGE_ACTION: - if (!key.empty()) { - return l10n_util::GetStringFUTF16( - IDS_EXTENSION_INSTALLED_PAGE_ACTION_INFO_WITH_SHORTCUT, key); - } else { - return l10n_util::GetStringUTF16( - IDS_EXTENSION_INSTALLED_PAGE_ACTION_INFO); - } - break; - case ExtensionInstalledBubble::OMNIBOX_KEYWORD: - return l10n_util::GetStringFUTF16( - IDS_EXTENSION_INSTALLED_OMNIBOX_KEYWORD_INFO, key); - break; - default: - NOTREACHED(); - break; - } - return base::string16(); -} - int InstalledBubbleContent::LayoutSigninPromo(int offset_x, int offset_y) { sign_in_promo_lines_.clear(); int height = 0; @@ -589,9 +510,7 @@ bool ExtensionInstalledBubbleView::MaybeShowNow() { views::BubbleBorder::TOP_LEFT : views::BubbleBorder::TOP_RIGHT); SetLayoutManager(new views::FillLayout()); - AddChildView(new InstalledBubbleContent( - bubble_.browser(), bubble_.extension(), bubble_.type(), - &bubble_.icon())); + AddChildView(new InstalledBubbleContent(bubble_, bubble_.browser())); views::BubbleDelegateView::CreateBubble(this)->Show(); |