Commit fbd98675 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

cbui extensions: further break deps on ExtensionInstalledBubble

This change breaks deps on:

* ExtensionInstalledBubble::options()
* ExtensionInstalledBubble::browser()
* ExtensionInstalledBubble::GetAnchorPoint()

The options one is annoying - there is a function called
`GetOptionsForExtension` that computed a bitfield here to be passed into the
view itself. This change replaces that function and that bitfield with a bunch
of free functions, each of which compute one of the option values. I think the
overall result is easier to reason about.

The browser one is straightforward.

The GetAnchorPoint one was dead code in Views anyway - the implementation
actually lives in this file in //cbui/v and not with the rest of the class, and
it simply does:

  gfx::Point ExtensionInstalledBubble::GetAnchorPoint(
      gfx::NativeWindow window) const {
    NOTREACHED();  // There is always an anchor view.
    return gfx::Point();
  }

So the code now checks and relies on that assumption.

Bug: 496955
Change-Id: Ic5d32d33bb6f50a02d95e46c4d937605f2aa88b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1958827
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723360}
parent 848ed5ec
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "build/buildflag.h" #include "build/buildflag.h"
#include "chrome/browser/extensions/api/commands/command_service.h"
#include "chrome/browser/extensions/extension_action_manager.h" #include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/platform_util.h" #include "chrome/browser/platform_util.h"
#include "chrome/browser/signin/signin_ui_util.h" #include "chrome/browser/signin/signin_ui_util.h"
...@@ -23,6 +24,7 @@ ...@@ -23,6 +24,7 @@
#include "chrome/browser/ui/extensions/extension_installed_bubble.h" #include "chrome/browser/ui/extensions/extension_installed_bubble.h"
#include "chrome/browser/ui/singleton_tabs.h" #include "chrome/browser/ui/singleton_tabs.h"
#include "chrome/browser/ui/sync/bubble_sync_promo_delegate.h" #include "chrome/browser/ui/sync/bubble_sync_promo_delegate.h"
#include "chrome/browser/ui/sync/sync_promo_ui.h"
#include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h" #include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h" #include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
...@@ -34,6 +36,8 @@ ...@@ -34,6 +36,8 @@
#include "chrome/browser/ui/views/toolbar/browser_actions_container.h" #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/common/extensions/api/omnibox/omnibox_handler.h" #include "chrome/common/extensions/api/omnibox/omnibox_handler.h"
#include "chrome/common/extensions/command.h"
#include "chrome/common/extensions/sync_helper.h"
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "chrome/grit/chromium_strings.h" #include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -134,16 +138,12 @@ views::View* AnchorViewForBrowser(const extensions::Extension* extension, ...@@ -134,16 +138,12 @@ views::View* AnchorViewForBrowser(const extensions::Extension* extension,
} }
std::unique_ptr<views::View> CreateSigninPromoView( std::unique_ptr<views::View> CreateSigninPromoView(
int bubble_options,
Profile* profile, Profile* profile,
BubbleSyncPromoDelegate* delegate) { BubbleSyncPromoDelegate* delegate) {
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
// ChromeOS does not show the signin promo. // ChromeOS does not show the signin promo.
return nullptr; return nullptr;
#else #else
if (!(bubble_options & ExtensionInstalledBubble::SIGN_IN_PROMO))
return nullptr;
return std::make_unique<DiceBubbleSyncPromoView>( return std::make_unique<DiceBubbleSyncPromoView>(
profile, delegate, profile, delegate,
signin_metrics::AccessPoint::ACCESS_POINT_EXTENSION_INSTALL_BUBBLE, signin_metrics::AccessPoint::ACCESS_POINT_EXTENSION_INSTALL_BUBBLE,
...@@ -165,6 +165,79 @@ gfx::ImageSkia MakeIconFromBitmap(const SkBitmap& bitmap) { ...@@ -165,6 +165,79 @@ gfx::ImageSkia MakeIconFromBitmap(const SkBitmap& bitmap) {
skia::ImageOperations::RESIZE_BEST, size); skia::ImageOperations::RESIZE_BEST, size);
} }
bool HasOmniboxKeyword(const Extension* extension) {
return extensions::OmniboxInfo::GetKeyword(extension).empty();
}
bool ShouldShowHowToUse(const extensions::Extension* extension) {
const auto* info = GetActionInfoForExtension(extension);
if (!info)
return false;
switch (info->type) {
case extensions::ActionInfo::TYPE_BROWSER:
case extensions::ActionInfo::TYPE_PAGE:
return !info->synthesized;
case extensions::ActionInfo::TYPE_ACTION:
return HasOmniboxKeyword(extension);
}
}
bool HasCommandKeybinding(const extensions::Extension* extension,
const Browser* browser) {
const auto* info = GetActionInfoForExtension(extension);
extensions::CommandService* command_service =
extensions::CommandService::Get(browser->profile());
extensions::Command command;
if (info->type == extensions::ActionInfo::TYPE_BROWSER) {
return command_service->GetBrowserActionCommand(
extension->id(), extensions::CommandService::ACTIVE, &command, nullptr);
} else if (info->type == extensions::ActionInfo::TYPE_PAGE) {
return command_service->GetPageActionCommand(
extension->id(), extensions::CommandService::ACTIVE, &command, nullptr);
}
return false;
}
bool ShouldShowHowToManage(const extensions::Extension* extension,
const Browser* browser) {
const auto* info = GetActionInfoForExtension(extension);
if (!info)
return false;
switch (info->type) {
case extensions::ActionInfo::TYPE_BROWSER:
case extensions::ActionInfo::TYPE_PAGE:
return !HasCommandKeybinding(extension, browser);
case extensions::ActionInfo::TYPE_ACTION:
return HasOmniboxKeyword(extension);
}
}
bool ShouldShowKeybinding(const Extension* extension, const Browser* browser) {
const auto* info = GetActionInfoForExtension(extension);
if (!info)
return false;
switch (info->type) {
case extensions::ActionInfo::TYPE_BROWSER:
case extensions::ActionInfo::TYPE_PAGE:
return HasCommandKeybinding(extension, browser);
case extensions::ActionInfo::TYPE_ACTION:
return false;
}
}
bool ShouldShowSignInPromo(const Extension* extension, const Browser* browser) {
return extensions::sync_helper::IsSyncable(extension) &&
SyncPromoUI::ShouldShowSyncPromo(browser->profile());
}
} // namespace } // namespace
// Provides feedback to the user upon successful installation of an // Provides feedback to the user upon successful installation of an
...@@ -183,6 +256,7 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate, ...@@ -183,6 +256,7 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate,
ExtensionInstalledBubbleView( ExtensionInstalledBubbleView(
ExtensionInstalledBubble* bubble, ExtensionInstalledBubble* bubble,
BubbleReference reference, BubbleReference reference,
Browser* browser,
scoped_refptr<const extensions::Extension> extension, scoped_refptr<const extensions::Extension> extension,
const SkBitmap& icon); const SkBitmap& icon);
~ExtensionInstalledBubbleView() override; ~ExtensionInstalledBubbleView() override;
...@@ -193,8 +267,6 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate, ...@@ -193,8 +267,6 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate,
void CloseBubble(BubbleCloseReason reason); void CloseBubble(BubbleCloseReason reason);
private: private:
Browser* browser() { return controller_->browser(); }
// views::BubbleDialogDelegateView: // views::BubbleDialogDelegateView:
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
gfx::ImageSkia GetWindowIcon() override; gfx::ImageSkia GetWindowIcon() override;
...@@ -216,6 +288,7 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate, ...@@ -216,6 +288,7 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate,
// The shortcut to open the manage shortcuts page. // The shortcut to open the manage shortcuts page.
views::Link* manage_shortcut_; views::Link* manage_shortcut_;
Browser* const browser_;
const scoped_refptr<const extensions::Extension> extension_; const scoped_refptr<const extensions::Extension> extension_;
const gfx::ImageSkia icon_; const gfx::ImageSkia icon_;
...@@ -225,6 +298,7 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate, ...@@ -225,6 +298,7 @@ class ExtensionInstalledBubbleView : public BubbleSyncPromoDelegate,
ExtensionInstalledBubbleView::ExtensionInstalledBubbleView( ExtensionInstalledBubbleView::ExtensionInstalledBubbleView(
ExtensionInstalledBubble* controller, ExtensionInstalledBubble* controller,
BubbleReference bubble_reference, BubbleReference bubble_reference,
Browser* browser,
scoped_refptr<const extensions::Extension> extension, scoped_refptr<const extensions::Extension> extension,
const SkBitmap& icon) const SkBitmap& icon)
: BubbleDialogDelegateView(nullptr, : BubbleDialogDelegateView(nullptr,
...@@ -235,27 +309,24 @@ ExtensionInstalledBubbleView::ExtensionInstalledBubbleView( ...@@ -235,27 +309,24 @@ ExtensionInstalledBubbleView::ExtensionInstalledBubbleView(
controller_(controller), controller_(controller),
bubble_reference_(bubble_reference), bubble_reference_(bubble_reference),
manage_shortcut_(nullptr), manage_shortcut_(nullptr),
browser_(browser),
extension_(extension), extension_(extension),
icon_(MakeIconFromBitmap(icon)) { icon_(MakeIconFromBitmap(icon)) {
chrome::RecordDialogCreation(chrome::DialogIdentifier::EXTENSION_INSTALLED); chrome::RecordDialogCreation(chrome::DialogIdentifier::EXTENSION_INSTALLED);
DialogDelegate::set_buttons(ui::DIALOG_BUTTON_NONE); DialogDelegate::set_buttons(ui::DIALOG_BUTTON_NONE);
DialogDelegate::SetFootnoteView(CreateSigninPromoView( if (ShouldShowSignInPromo(extension_.get(), browser_)) {
controller->options(), controller->browser()->profile(), this)); DialogDelegate::SetFootnoteView(
CreateSigninPromoView(browser->profile(), this));
}
} }
ExtensionInstalledBubbleView::~ExtensionInstalledBubbleView() {} ExtensionInstalledBubbleView::~ExtensionInstalledBubbleView() {}
void ExtensionInstalledBubbleView::UpdateAnchorView() { void ExtensionInstalledBubbleView::UpdateAnchorView() {
views::View* reference_view = views::View* reference_view =
AnchorViewForBrowser(extension_.get(), browser()); AnchorViewForBrowser(extension_.get(), browser_);
if (reference_view) { DCHECK(reference_view);
SetAnchorView(reference_view); SetAnchorView(reference_view);
} else {
gfx::NativeWindow parent_window = browser()->window()->GetNativeWindow();
set_parent_window(platform_util::GetViewForWindow(parent_window));
gfx::Point window_offset = controller_->GetAnchorPoint(parent_window);
SetAnchorRect(gfx::Rect(window_offset, gfx::Size()));
}
} }
void ExtensionInstalledBubbleView::CloseBubble(BubbleCloseReason reason) { void ExtensionInstalledBubbleView::CloseBubble(BubbleCloseReason reason) {
...@@ -323,10 +394,10 @@ void ExtensionInstalledBubbleView::Init() { ...@@ -323,10 +394,10 @@ void ExtensionInstalledBubbleView::Init() {
views::BoxLayout::CrossAxisAlignment::kStart); views::BoxLayout::CrossAxisAlignment::kStart);
SetLayoutManager(std::move(layout)); SetLayoutManager(std::move(layout));
if (controller_->options() & ExtensionInstalledBubble::HOW_TO_USE) if (ShouldShowHowToUse(extension_.get()))
AddChildView(CreateLabel(controller_->GetHowToUseDescription())); AddChildView(CreateLabel(controller_->GetHowToUseDescription()));
if (controller_->options() & ExtensionInstalledBubble::SHOW_KEYBINDING) { if (ShouldShowKeybinding(extension_.get(), browser_)) {
manage_shortcut_ = new views::Link( manage_shortcut_ = new views::Link(
l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALLED_MANAGE_SHORTCUTS)); l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALLED_MANAGE_SHORTCUTS));
manage_shortcut_->set_listener(this); manage_shortcut_->set_listener(this);
...@@ -334,7 +405,7 @@ void ExtensionInstalledBubbleView::Init() { ...@@ -334,7 +405,7 @@ void ExtensionInstalledBubbleView::Init() {
AddChildView(manage_shortcut_); AddChildView(manage_shortcut_);
} }
if (controller_->options() & ExtensionInstalledBubble::HOW_TO_MANAGE) { if (ShouldShowHowToManage(extension_.get(), browser_)) {
AddChildView(CreateLabel( AddChildView(CreateLabel(
l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALLED_MANAGE_INFO))); l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALLED_MANAGE_INFO)));
} }
...@@ -343,7 +414,7 @@ void ExtensionInstalledBubbleView::Init() { ...@@ -343,7 +414,7 @@ void ExtensionInstalledBubbleView::Init() {
void ExtensionInstalledBubbleView::OnEnableSync(const AccountInfo& account, void ExtensionInstalledBubbleView::OnEnableSync(const AccountInfo& account,
bool is_default_promo_account) { bool is_default_promo_account) {
signin_ui_util::EnableSyncFromPromo( signin_ui_util::EnableSyncFromPromo(
browser(), account, browser_, account,
signin_metrics::AccessPoint::ACCESS_POINT_EXTENSION_INSTALL_BUBBLE, signin_metrics::AccessPoint::ACCESS_POINT_EXTENSION_INSTALL_BUBBLE,
is_default_promo_account); is_default_promo_account);
CloseBubble(BUBBLE_CLOSE_NAVIGATED); CloseBubble(BUBBLE_CLOSE_NAVIGATED);
...@@ -356,7 +427,7 @@ void ExtensionInstalledBubbleView::LinkClicked(views::Link* source, ...@@ -356,7 +427,7 @@ void ExtensionInstalledBubbleView::LinkClicked(views::Link* source,
std::string configure_url = chrome::kChromeUIExtensionsURL; std::string configure_url = chrome::kChromeUIExtensionsURL;
configure_url += chrome::kExtensionConfigureCommandsSubPage; configure_url += chrome::kExtensionConfigureCommandsSubPage;
NavigateParams params( NavigateParams params(
GetSingletonTabNavigateParams(browser(), GURL(configure_url))); GetSingletonTabNavigateParams(browser_, GURL(configure_url)));
Navigate(&params); Navigate(&params);
CloseBubble(BUBBLE_CLOSE_NAVIGATED); CloseBubble(BUBBLE_CLOSE_NAVIGATED);
} }
...@@ -374,7 +445,8 @@ ExtensionInstalledBubbleUi::~ExtensionInstalledBubbleUi() { ...@@ -374,7 +445,8 @@ ExtensionInstalledBubbleUi::~ExtensionInstalledBubbleUi() {
void ExtensionInstalledBubbleUi::Show(BubbleReference bubble_reference) { void ExtensionInstalledBubbleUi::Show(BubbleReference bubble_reference) {
bubble_view_ = new ExtensionInstalledBubbleView( bubble_view_ = new ExtensionInstalledBubbleView(
bubble_, bubble_reference, bubble_->extension(), bubble_->icon()); bubble_, bubble_reference, bubble_->browser(), bubble_->extension(),
bubble_->icon());
bubble_reference_ = bubble_reference; bubble_reference_ = bubble_reference;
views::BubbleDialogDelegateView::CreateBubble(bubble_view_)->Show(); views::BubbleDialogDelegateView::CreateBubble(bubble_view_)->Show();
...@@ -407,7 +479,7 @@ bool ExtensionInstalledBubble::ShouldShow() { ...@@ -407,7 +479,7 @@ bool ExtensionInstalledBubble::ShouldShow() {
return true; return true;
if (anchor_position() == ANCHOR_ACTION) { if (anchor_position() == ANCHOR_ACTION) {
BrowserActionsContainer* container = BrowserActionsContainer* container =
BrowserView::GetBrowserViewForBrowser(browser()) BrowserView::GetBrowserViewForBrowser(browser_)
->toolbar() ->toolbar()
->browser_actions(); ->browser_actions();
return container && !container->animating(); return container && !container->animating();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment