Commit 09c72290 authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

Migrate ImeWarningBubbleView to use container

This makes sure the ImeWarningBubbleView can show (and not crash :))
when ExtensionsToolbarMenu is enabled.

Also fixes ImeWarningBubbleTest.* when ExtensionsToolbarMenu is enabled.

Bug: chromium:943702, chromium:984654
Change-Id: Idf8bdd14792201eba421547b8ccd0543817e8ce8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2028186
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737188}
parent 765ee5c4
...@@ -7,9 +7,14 @@ ...@@ -7,9 +7,14 @@
#include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h" #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/extensions/extensions_toolbar_container.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/browser/ui/views/ime/ime_warning_bubble_view.h" #include "chrome/browser/ui/views/ime/ime_warning_bubble_view.h"
#include "ui/compositor/scoped_animation_duration_scale_mode.h"
#include "ui/views/controls/button/checkbox.h" #include "ui/views/controls/button/checkbox.h"
#include "ui/views/layout/animating_layout_manager_test_util.h"
class ImeWarningBubbleTest : public extensions::ExtensionBrowserTest { class ImeWarningBubbleTest : public extensions::ExtensionBrowserTest {
public: public:
...@@ -30,6 +35,9 @@ class ImeWarningBubbleTest : public extensions::ExtensionBrowserTest { ...@@ -30,6 +35,9 @@ class ImeWarningBubbleTest : public extensions::ExtensionBrowserTest {
bool ok_button_pressed_; bool ok_button_pressed_;
bool never_show_checked_; bool never_show_checked_;
ui::ScopedAnimationDurationScaleMode disable_animation_{
ui::ScopedAnimationDurationScaleMode::ZERO_DURATION};
DISALLOW_COPY_AND_ASSIGN(ImeWarningBubbleTest); DISALLOW_COPY_AND_ASSIGN(ImeWarningBubbleTest);
}; };
...@@ -48,6 +56,14 @@ void ImeWarningBubbleTest::SetUpOnMainThread() { ...@@ -48,6 +56,14 @@ void ImeWarningBubbleTest::SetUpOnMainThread() {
base::Unretained(this)); base::Unretained(this));
browser()->window()->ShowImeWarningBubble(extension_, callback_); browser()->window()->ShowImeWarningBubble(extension_, callback_);
ime_warning_bubble_ = ImeWarningBubbleView::ime_warning_bubble_for_test_; ime_warning_bubble_ = ImeWarningBubbleView::ime_warning_bubble_for_test_;
if (base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) {
// Wait for the animation to finish so that the bubble is visible.
views::test::WaitForAnimatingLayoutManager(
BrowserView::GetBrowserViewForBrowser(browser())
->toolbar_button_provider()
->GetExtensionsToolbarContainer());
}
} }
void ImeWarningBubbleTest::OnPermissionBubbleFinished( void ImeWarningBubbleTest::OnPermissionBubbleFinished(
......
...@@ -11,8 +11,10 @@ ...@@ -11,8 +11,10 @@
#include "chrome/browser/platform_util.h" #include "chrome/browser/platform_util.h"
#include "chrome/browser/ui/browser_dialogs.h" #include "chrome/browser/ui/browser_dialogs.h"
#include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_list.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/chrome_typography.h" #include "chrome/browser/ui/views/chrome_typography.h"
#include "chrome/browser/ui/views/extensions/extensions_toolbar_container.h"
#include "chrome/browser/ui/views/frame/app_menu_button.h" #include "chrome/browser/ui/views/frame/app_menu_button.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h" #include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
...@@ -93,8 +95,6 @@ ImeWarningBubbleView::ImeWarningBubbleView( ...@@ -93,8 +95,6 @@ ImeWarningBubbleView::ImeWarningBubbleView(
browser_view_(browser_view), browser_view_(browser_view),
browser_(browser_view->browser()), browser_(browser_view->browser()),
response_callback_(callback) { response_callback_(callback) {
container_ = browser_view_->toolbar()->browser_actions();
toolbar_actions_bar_ = container_->toolbar_actions_bar();
BrowserList::AddObserver(this); BrowserList::AddObserver(this);
// The lifetime of this bubble is tied to the lifetime of the browser. // The lifetime of this bubble is tied to the lifetime of the browser.
...@@ -103,10 +103,32 @@ ImeWarningBubbleView::ImeWarningBubbleView( ...@@ -103,10 +103,32 @@ ImeWarningBubbleView::ImeWarningBubbleView(
InitAnchorView(); InitAnchorView();
InitLayout(); InitLayout();
if (base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) {
// TODO(pbos): During cleanup (default-enabling this), remove
// OnBrowserRemoved and stop observing the browser. The widget will now
// always be created and always have ownership.
// TODO(pbos): Move widget creation outside this class when this is being
// cleaned up. ::ShowBubble should create the Widget and queue showing it.
bubble_has_shown_ = true;
ExtensionsToolbarContainer* const container =
browser_view_->toolbar_button_provider()
->GetExtensionsToolbarContainer();
views::Widget* const widget =
views::BubbleDialogDelegateView::CreateBubble(this);
if (container) {
container->ShowWidgetForExtension(widget, extension_->id());
} else {
widget->Show();
}
chrome::RecordDialogCreation(chrome::DialogIdentifier::IME_WARNING);
return;
}
// If the toolbar is not animating, shows the warning bubble directly. // If the toolbar is not animating, shows the warning bubble directly.
// Otherwise, shows the bubble in method OnToolbarActionsBarAnimationEnded(). // Otherwise, shows the bubble in method OnToolbarActionsBarAnimationEnded().
if (IsToolbarAnimating()) { if (IsToolbarAnimating()) {
toolbar_actions_bar_observer_.Add(toolbar_actions_bar_); toolbar_actions_bar_observer_.Add(
browser_view_->toolbar()->browser_actions()->toolbar_actions_bar());
return; return;
} }
views::BubbleDialogDelegateView::CreateBubble(this)->Show(); views::BubbleDialogDelegateView::CreateBubble(this)->Show();
...@@ -124,20 +146,29 @@ ImeWarningBubbleView::~ImeWarningBubbleView() { ...@@ -124,20 +146,29 @@ ImeWarningBubbleView::~ImeWarningBubbleView() {
} }
void ImeWarningBubbleView::InitAnchorView() { void ImeWarningBubbleView::InitAnchorView() {
views::View* reference_view = nullptr; views::View* anchor_view = nullptr;
anchor_to_action_ =
extensions::ActionInfo::GetAnyActionInfo(extension_) != nullptr;
if (anchor_to_action_) { if (anchor_to_action_) {
// Anchors the bubble to the browser action of the extension. ExtensionsToolbarContainer* const container =
reference_view = container_->GetViewForId(extension_->id()); browser_view_->toolbar_button_provider()
->GetExtensionsToolbarContainer();
if (container) {
anchor_view = container->GetViewForId(extension_->id());
} else if (!base::FeatureList::IsEnabled(
features::kExtensionsToolbarMenu)) {
BrowserActionsContainer* const browser_actions_container =
browser_view_->toolbar_button_provider()
->GetBrowserActionsContainer();
ToolbarActionView* const reference_view =
browser_actions_container->GetViewForId(extension_->id());
if (reference_view && reference_view->GetVisible())
anchor_view = reference_view;
}
} }
if (!reference_view || !reference_view->GetVisible()) { if (!anchor_view) {
// Anchors the bubble to the app menu. anchor_view = browser_view_->toolbar_button_provider()
reference_view = ->GetDefaultExtensionDialogAnchorView();
browser_view_->toolbar_button_provider()->GetAppMenuButton();
} }
SetAnchorView(reference_view); SetAnchorView(anchor_view);
SetArrow(views::BubbleBorder::TOP_RIGHT); SetArrow(views::BubbleBorder::TOP_RIGHT);
} }
...@@ -185,5 +216,7 @@ void ImeWarningBubbleView::InitLayout() { ...@@ -185,5 +216,7 @@ void ImeWarningBubbleView::InitLayout() {
} }
bool ImeWarningBubbleView::IsToolbarAnimating() { bool ImeWarningBubbleView::IsToolbarAnimating() {
return anchor_to_action_ && container_->animating(); DCHECK(!base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu));
return anchor_to_action_ &&
browser_view_->toolbar()->browser_actions()->animating();
} }
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "chrome/browser/ui/toolbar/toolbar_actions_bar_observer.h" #include "chrome/browser/ui/toolbar/toolbar_actions_bar_observer.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h" #include "ui/views/bubble/bubble_dialog_delegate_view.h"
class BrowserActionsContainer;
class BrowserView; class BrowserView;
enum class ImeWarningBubblePermissionStatus; enum class ImeWarningBubblePermissionStatus;
...@@ -69,7 +68,7 @@ class ImeWarningBubbleView : public views::BubbleDialogDelegateView, ...@@ -69,7 +68,7 @@ class ImeWarningBubbleView : public views::BubbleDialogDelegateView,
bool IsToolbarAnimating(); bool IsToolbarAnimating();
const extensions::Extension* extension_; const extensions::Extension* extension_;
BrowserView* browser_view_; BrowserView* const browser_view_;
// Saves the Browser instance of the browser view, which will be used in // Saves the Browser instance of the browser view, which will be used in
// OnBrowserRemoved(), as browser_view_->browser() may be null when // OnBrowserRemoved(), as browser_view_->browser() may be null when
...@@ -87,10 +86,6 @@ class ImeWarningBubbleView : public views::BubbleDialogDelegateView, ...@@ -87,10 +86,6 @@ class ImeWarningBubbleView : public views::BubbleDialogDelegateView,
// True if the warning bubble has been shown. // True if the warning bubble has been shown.
bool bubble_has_shown_ = false; bool bubble_has_shown_ = false;
BrowserActionsContainer* container_;
ToolbarActionsBar* toolbar_actions_bar_;
ScopedObserver<ToolbarActionsBar, ToolbarActionsBarObserver> ScopedObserver<ToolbarActionsBar, ToolbarActionsBarObserver>
toolbar_actions_bar_observer_{this}; toolbar_actions_bar_observer_{this};
......
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