diff options
author | vasilii <vasilii@chromium.org> | 2015-04-23 02:12:06 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-23 09:12:18 +0000 |
commit | 11e724cae6142ebb800a796fc8d4d36f0aa55c68 (patch) | |
tree | 53a44947a4e87d1d9d70d69f34597de00eba6e3f | |
parent | 98c544c39d429fe7d27d86337790f26ce74e782f (diff) | |
download | chromium_src-11e724cae6142ebb800a796fc8d4d36f0aa55c68.zip chromium_src-11e724cae6142ebb800a796fc8d4d36f0aa55c68.tar.gz chromium_src-11e724cae6142ebb800a796fc8d4d36f0aa55c68.tar.bz2 |
Move the omnibox bubbles when their anchor changes position.
Zoom, Bookmarks, Translate and Password icon can change position because they don't appear simultaneously. In this case the corresponding bubble should shift too.
BUG=474646
Review URL: https://codereview.chromium.org/1070073002
Cr-Commit-Position: refs/heads/master@{#326482}
25 files changed, 133 insertions, 136 deletions
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc index 98a9bc6..9808dc1 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc +++ b/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc @@ -68,7 +68,7 @@ void BookmarkBubbleView::ShowBubble(views::View* anchor_view, Profile* profile, const GURL& url, bool newly_bookmarked) { - if (IsShowing()) + if (bookmark_bubble_) return; bookmark_bubble_ = new BookmarkBubbleView(anchor_view, @@ -86,13 +86,8 @@ void BookmarkBubbleView::ShowBubble(views::View* anchor_view, bookmark_bubble_->observer_->OnBookmarkBubbleShown(url); } -// static -bool BookmarkBubbleView::IsShowing() { - return bookmark_bubble_ != NULL; -} - void BookmarkBubbleView::Hide() { - if (IsShowing()) + if (bookmark_bubble_) bookmark_bubble_->GetWidget()->Close(); } diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h b/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h index 08e46d9..51892ab 100644 --- a/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h +++ b/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h @@ -40,10 +40,10 @@ class BookmarkBubbleView : public views::BubbleDelegateView, const GURL& url, bool newly_bookmarked); - static bool IsShowing(); - static void Hide(); + static BookmarkBubbleView* bookmark_bubble() { return bookmark_bubble_; } + ~BookmarkBubbleView() override; // views::WidgetDelegate: diff --git a/chrome/browser/ui/views/extensions/bookmark_override_browsertest.cc b/chrome/browser/ui/views/extensions/bookmark_override_browsertest.cc index c61b1e5..aa3f4ae 100644 --- a/chrome/browser/ui/views/extensions/bookmark_override_browsertest.cc +++ b/chrome/browser/ui/views/extensions/bookmark_override_browsertest.cc @@ -58,11 +58,11 @@ IN_PROC_BROWSER_TEST_F(BookmarkOverrideTest, DISABLED_NonOverrideStarClick) { ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON); // Verify that clicking once shows the bookmark bubble. - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); star_view->OnMousePressed(pressed_event); - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); star_view->OnMouseReleased(released_event); - EXPECT_TRUE(BookmarkBubbleView::IsShowing()); + EXPECT_TRUE(BookmarkBubbleView::bookmark_bubble()); } // Test that invoking the IDC_BOOKMARK_PAGE command (as done by the wrench menu) @@ -84,7 +84,7 @@ IN_PROC_BROWSER_TEST_F(BookmarkOverrideTest, DISABLED_NonOverrideBookmarkPage) { // Check that the BookmarkBubbleView is shown when executing // IDC_BOOKMARK_PAGE. - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); chrome::ExecuteCommand(browser(), IDC_BOOKMARK_PAGE); - EXPECT_TRUE(BookmarkBubbleView::IsShowing()); + EXPECT_TRUE(BookmarkBubbleView::bookmark_bubble()); } diff --git a/chrome/browser/ui/views/location_bar/bubble_icon_view.cc b/chrome/browser/ui/views/location_bar/bubble_icon_view.cc index d74f329..8d49ced 100644 --- a/chrome/browser/ui/views/location_bar/bubble_icon_view.cc +++ b/chrome/browser/ui/views/location_bar/bubble_icon_view.cc @@ -7,6 +7,7 @@ #include "chrome/browser/command_updater.h" #include "ui/accessibility/ax_view_state.h" #include "ui/events/event.h" +#include "ui/views/bubble/bubble_delegate.h" BubbleIconView::BubbleIconView(CommandUpdater* command_updater, int command_id) : command_updater_(command_updater), @@ -18,6 +19,12 @@ BubbleIconView::BubbleIconView(CommandUpdater* command_updater, int command_id) BubbleIconView::~BubbleIconView() { } +bool BubbleIconView::IsBubbleShowing() const { + // If the bubble is being destroyed, it's considered showing though it may be + // already invisible currently. + return GetBubble() != NULL; +} + void BubbleIconView::GetAccessibleState(ui::AXViewState* state) { views::ImageView::GetAccessibleState(state); state->role = ui::AX_ROLE_BUTTON; @@ -74,3 +81,9 @@ void BubbleIconView::ExecuteCommand(ExecuteSource source) { if (command_updater_) command_updater_->ExecuteCommand(command_id_); } + +void BubbleIconView::OnBoundsChanged(const gfx::Rect& previous_bounds) { + views::BubbleDelegateView* bubble = GetBubble(); + if (bubble) + bubble->OnAnchorBoundsChanged(); +} diff --git a/chrome/browser/ui/views/location_bar/bubble_icon_view.h b/chrome/browser/ui/views/location_bar/bubble_icon_view.h index 504cf87..94e6143 100644 --- a/chrome/browser/ui/views/location_bar/bubble_icon_view.h +++ b/chrome/browser/ui/views/location_bar/bubble_icon_view.h @@ -9,6 +9,10 @@ class CommandUpdater; +namespace views { +class BubbleDelegateView; +} + // Represents an icon on the omnibox that shows a bubble when clicked. class BubbleIconView : public views::ImageView { protected: @@ -22,7 +26,7 @@ class BubbleIconView : public views::ImageView { ~BubbleIconView() override; // Returns true if a related bubble is showing. - virtual bool IsBubbleShowing() const = 0; + bool IsBubbleShowing() const; // Invoked prior to executing the command. virtual void OnExecuting(ExecuteSource execute_source) = 0; @@ -42,6 +46,12 @@ class BubbleIconView : public views::ImageView { // Calls OnExecuting and runs |command_id_| with a valid |command_updater_|. virtual void ExecuteCommand(ExecuteSource source); + // Returns the bubble instance for the icon. + virtual views::BubbleDelegateView* GetBubble() const = 0; + + // views::View: + void OnBoundsChanged(const gfx::Rect& previous_bounds) override; + private: // The CommandUpdater for the Browser object that owns the location bar. CommandUpdater* command_updater_; diff --git a/chrome/browser/ui/views/location_bar/star_view.cc b/chrome/browser/ui/views/location_bar/star_view.cc index 368119d..080d525 100644 --- a/chrome/browser/ui/views/location_bar/star_view.cc +++ b/chrome/browser/ui/views/location_bar/star_view.cc @@ -32,10 +32,6 @@ void StarView::SetToggled(bool on) { on ? IDR_STAR_LIT : IDR_STAR)); } -bool StarView::IsBubbleShowing() const { - return BookmarkBubbleView::IsShowing(); -} - void StarView::OnExecuting( BubbleIconView::ExecuteSource execute_source) { BookmarkEntryPoint entry_point = BOOKMARK_ENTRY_POINT_STAR_MOUSE; @@ -63,3 +59,7 @@ void StarView::ExecuteCommand(ExecuteSource source) { BubbleIconView::ExecuteCommand(source); } } + +views::BubbleDelegateView* StarView::GetBubble() const { + return BookmarkBubbleView::bookmark_bubble(); +} diff --git a/chrome/browser/ui/views/location_bar/star_view.h b/chrome/browser/ui/views/location_bar/star_view.h index 1b0e63c..57da0f2 100644 --- a/chrome/browser/ui/views/location_bar/star_view.h +++ b/chrome/browser/ui/views/location_bar/star_view.h @@ -21,9 +21,9 @@ class StarView : public BubbleIconView { protected: // BubbleIconView: - bool IsBubbleShowing() const override; void OnExecuting(BubbleIconView::ExecuteSource execute_source) override; void ExecuteCommand(ExecuteSource source) override; + views::BubbleDelegateView* GetBubble() const override; private: Browser* browser_; diff --git a/chrome/browser/ui/views/location_bar/star_view_browsertest.cc b/chrome/browser/ui/views/location_bar/star_view_browsertest.cc index 717562b..f5c620d 100644 --- a/chrome/browser/ui/views/location_bar/star_view_browsertest.cc +++ b/chrome/browser/ui/views/location_bar/star_view_browsertest.cc @@ -50,11 +50,11 @@ IN_PROC_BROWSER_TEST_F(StarViewTest, MAYBE_HideOnSecondClick) { ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON); // Verify that clicking once shows the bookmark bubble. - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); star_view->OnMousePressed(pressed_event); - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); star_view->OnMouseReleased(released_event); - EXPECT_TRUE(BookmarkBubbleView::IsShowing()); + EXPECT_TRUE(BookmarkBubbleView::bookmark_bubble()); // Verify that clicking again doesn't reshow it. star_view->OnMousePressed(pressed_event); @@ -62,9 +62,9 @@ IN_PROC_BROWSER_TEST_F(StarViewTest, MAYBE_HideOnSecondClick) { // the event processing. BookmarkBubbleView::Hide(); base::MessageLoop::current()->RunUntilIdle(); - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); star_view->OnMouseReleased(released_event); - EXPECT_FALSE(BookmarkBubbleView::IsShowing()); + EXPECT_FALSE(BookmarkBubbleView::bookmark_bubble()); } #if defined(OS_WIN) @@ -138,7 +138,7 @@ IN_PROC_BROWSER_TEST_F(StarViewTestNoDWM, DISABLED_WindowedNPAPIPluginHidden) { runner->QuitClosure()); runner->Run(); - EXPECT_TRUE(BookmarkBubbleView::IsShowing()); + EXPECT_TRUE(BookmarkBubbleView::bookmark_bubble()); result = GetWindowRgnBox(child, ®ion_after); if (result == NULLREGION) { diff --git a/chrome/browser/ui/views/location_bar/translate_icon_view.cc b/chrome/browser/ui/views/location_bar/translate_icon_view.cc index 3a7add4..a73564f 100644 --- a/chrome/browser/ui/views/location_bar/translate_icon_view.cc +++ b/chrome/browser/ui/views/location_bar/translate_icon_view.cc @@ -29,10 +29,10 @@ void TranslateIconView::SetToggled(bool on) { on ? IDR_TRANSLATE_ACTIVE : IDR_TRANSLATE)); } -bool TranslateIconView::IsBubbleShowing() const { - return TranslateBubbleView::IsShowing(); +void TranslateIconView::OnExecuting( + BubbleIconView::ExecuteSource execute_source) { } -void TranslateIconView::OnExecuting( - BubbleIconView::ExecuteSource execute_source) { +views::BubbleDelegateView* TranslateIconView::GetBubble() const { + return TranslateBubbleView::GetCurrentBubble(); } diff --git a/chrome/browser/ui/views/location_bar/translate_icon_view.h b/chrome/browser/ui/views/location_bar/translate_icon_view.h index 4ebafd3..bc57187 100644 --- a/chrome/browser/ui/views/location_bar/translate_icon_view.h +++ b/chrome/browser/ui/views/location_bar/translate_icon_view.h @@ -21,8 +21,8 @@ class TranslateIconView : public BubbleIconView { protected: // BubbleIconView: - bool IsBubbleShowing() const override; void OnExecuting(BubbleIconView::ExecuteSource execute_source) override; + views::BubbleDelegateView* GetBubble() const override; private: DISALLOW_COPY_AND_ASSIGN(TranslateIconView); diff --git a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc b/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc index eff898a..9308b6d 100644 --- a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc +++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc @@ -101,14 +101,7 @@ void ZoomBubbleView::CloseBubble() { } // static -bool ZoomBubbleView::IsShowing() { - // The bubble is considered showing while closing. - return zoom_bubble_ != NULL && (zoom_bubble_->GetWidget()->IsVisible() || - zoom_bubble_->GetWidget()->IsClosed()); -} - -// static -const ZoomBubbleView* ZoomBubbleView::GetZoomBubbleForTest() { +ZoomBubbleView* ZoomBubbleView::GetZoomBubble() { return zoom_bubble_; } diff --git a/chrome/browser/ui/views/location_bar/zoom_bubble_view.h b/chrome/browser/ui/views/location_bar/zoom_bubble_view.h index b8039e6..83bb3f3 100644 --- a/chrome/browser/ui/views/location_bar/zoom_bubble_view.h +++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view.h @@ -43,12 +43,9 @@ class ZoomBubbleView : public ManagedFullScreenBubbleDelegateView, // Closes the showing bubble (if one exists). static void CloseBubble(); - // Whether the zoom bubble is currently showing. - static bool IsShowing(); - // Returns the zoom bubble if the zoom bubble is showing. Returns NULL // otherwise. - static const ZoomBubbleView* GetZoomBubbleForTest(); + static ZoomBubbleView* GetZoomBubble(); private: FRIEND_TEST_ALL_PREFIXES(ZoomBubbleBrowserTest, ImmersiveFullscreen); diff --git a/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc b/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc index 207c029..db9e61c 100644 --- a/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc +++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc @@ -27,8 +27,8 @@ IN_PROC_BROWSER_TEST_F(ZoomBubbleBrowserTest, MAYBE_NonImmersiveFullscreen) { // The zoom bubble should be anchored when not in fullscreen. ZoomBubbleView::ShowBubble(web_contents, true); - ASSERT_TRUE(ZoomBubbleView::IsShowing()); - const ZoomBubbleView* zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + ASSERT_TRUE(ZoomBubbleView::GetZoomBubble()); + const ZoomBubbleView* zoom_bubble = ZoomBubbleView::GetZoomBubble(); EXPECT_TRUE(zoom_bubble->GetAnchorView()); // Entering fullscreen should close the bubble. (We enter into tab fullscreen @@ -45,13 +45,13 @@ IN_PROC_BROWSER_TEST_F(ZoomBubbleBrowserTest, MAYBE_NonImmersiveFullscreen) { waiter->Wait(); } ASSERT_FALSE(browser_view->immersive_mode_controller()->IsEnabled()); - EXPECT_FALSE(ZoomBubbleView::IsShowing()); + EXPECT_FALSE(ZoomBubbleView::GetZoomBubble()); // The bubble should not be anchored when it is shown in non-immersive // fullscreen. ZoomBubbleView::ShowBubble(web_contents, true); - ASSERT_TRUE(ZoomBubbleView::IsShowing()); - zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + ASSERT_TRUE(ZoomBubbleView::GetZoomBubble()); + zoom_bubble = ZoomBubbleView::GetZoomBubble(); EXPECT_FALSE(zoom_bubble->GetAnchorView()); // Exit fullscreen before ending the test for the sake of sanity. @@ -88,8 +88,8 @@ IN_PROC_BROWSER_TEST_F(ZoomBubbleBrowserTest, ImmersiveFullscreen) { // The zoom bubble should not be anchored when it is shown in immersive // fullscreen and the top-of-window views are not revealed. ZoomBubbleView::ShowBubble(web_contents, true); - ASSERT_TRUE(ZoomBubbleView::IsShowing()); - const ZoomBubbleView* zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + ASSERT_TRUE(ZoomBubbleView::GetZoomBubble()); + const ZoomBubbleView* zoom_bubble = ZoomBubbleView::GetZoomBubble(); EXPECT_FALSE(zoom_bubble->GetAnchorView()); // An immersive reveal should hide the zoom bubble. @@ -102,8 +102,8 @@ IN_PROC_BROWSER_TEST_F(ZoomBubbleBrowserTest, ImmersiveFullscreen) { // The zoom bubble should be anchored when it is shown in immersive fullscreen // and the top-of-window views are revealed. ZoomBubbleView::ShowBubble(web_contents, true); - ASSERT_TRUE(ZoomBubbleView::IsShowing()); - zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + ASSERT_TRUE(ZoomBubbleView::GetZoomBubble()); + zoom_bubble = ZoomBubbleView::GetZoomBubble(); EXPECT_TRUE(zoom_bubble->GetAnchorView()); // The top-of-window views should not hide till the zoom bubble hides. (It diff --git a/chrome/browser/ui/views/location_bar/zoom_view.cc b/chrome/browser/ui/views/location_bar/zoom_view.cc index 6883537..a2f79cd 100644 --- a/chrome/browser/ui/views/location_bar/zoom_view.cc +++ b/chrome/browser/ui/views/location_bar/zoom_view.cc @@ -47,10 +47,6 @@ void ZoomView::Update(ui_zoom::ZoomController* zoom_controller) { SetVisible(true); } -bool ZoomView::IsBubbleShowing() const { - return ZoomBubbleView::IsShowing(); -} - void ZoomView::OnExecuting(BubbleIconView::ExecuteSource source) { ZoomBubbleView::ShowBubble(location_bar_delegate_->GetWebContents(), false); } @@ -59,3 +55,7 @@ void ZoomView::GetAccessibleState(ui::AXViewState* state) { BubbleIconView::GetAccessibleState(state); state->name = l10n_util::GetStringUTF16(IDS_ACCNAME_ZOOM); } + +views::BubbleDelegateView* ZoomView::GetBubble() const { + return ZoomBubbleView::GetZoomBubble(); +} diff --git a/chrome/browser/ui/views/location_bar/zoom_view.h b/chrome/browser/ui/views/location_bar/zoom_view.h index 3ec315e..1224f8c 100644 --- a/chrome/browser/ui/views/location_bar/zoom_view.h +++ b/chrome/browser/ui/views/location_bar/zoom_view.h @@ -29,9 +29,9 @@ class ZoomView : public BubbleIconView { protected: // BubbleIconView: - bool IsBubbleShowing() const override; void OnExecuting(BubbleIconView::ExecuteSource source) override; void GetAccessibleState(ui::AXViewState* state) override; + views::BubbleDelegateView* GetBubble() const override; private: // The delegate used to get the currently visible WebContents. diff --git a/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc b/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc index 9a867d2..04e5cc5 100644 --- a/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc +++ b/chrome/browser/ui/views/managed_full_screen_bubble_delegate_view.cc @@ -41,7 +41,9 @@ void ManagedFullScreenBubbleDelegateView::Observe( } void ManagedFullScreenBubbleDelegateView::Close() { - GetWidget()->Close(); + views::Widget* widget = GetWidget(); + if (!widget->IsClosed()) + widget->Close(); } void ManagedFullScreenBubbleDelegateView::AdjustForFullscreen( diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc index b1bede3..0daf101 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc +++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc @@ -1058,9 +1058,8 @@ void ManagePasswordsBubbleView::ShowBubble(content::WebContents* web_contents, Browser* browser = chrome::FindBrowserWithWebContents(web_contents); DCHECK(browser); DCHECK(browser->window()); - - if (IsShowing()) - return; + DCHECK(!manage_passwords_bubble_ || + !manage_passwords_bubble_->GetWidget()->IsVisible()); BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser); bool is_fullscreen = browser_view->IsFullscreen(); @@ -1095,18 +1094,11 @@ void ManagePasswordsBubbleView::CloseBubble() { // static void ManagePasswordsBubbleView::ActivateBubble() { - if (!IsShowing()) - return; + DCHECK(manage_passwords_bubble_); + DCHECK(manage_passwords_bubble_->GetWidget()->IsVisible()); manage_passwords_bubble_->GetWidget()->Activate(); } -// static -bool ManagePasswordsBubbleView::IsShowing() { - // The bubble may be in the process of closing. - return (manage_passwords_bubble_ != NULL) && - manage_passwords_bubble_->GetWidget()->IsVisible(); -} - content::WebContents* ManagePasswordsBubbleView::web_contents() const { return model()->web_contents(); } diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h index 173b995..01c0ea1 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h +++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h @@ -35,11 +35,8 @@ class ManagePasswordsBubbleView : public ManagePasswordsBubble, // Makes the bubble the foreground window. static void ActivateBubble(); - // Whether the bubble is currently showing. - static bool IsShowing(); - // Returns a pointer to the bubble. - static const ManagePasswordsBubbleView* manage_password_bubble() { + static ManagePasswordsBubbleView* manage_password_bubble() { return manage_passwords_bubble_; } diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc index 654b1d56..2100e80 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc +++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc @@ -48,6 +48,12 @@ class TestURLFetcherCallback { MOCK_METHOD1(OnRequestDone, void(const GURL&)); }; +bool IsBubbleShowing() { + return ManagePasswordsBubbleView::manage_password_bubble() && + ManagePasswordsBubbleView::manage_password_bubble()-> + GetWidget()->IsVisible(); +} + } // namespace namespace metrics_util = password_manager::metrics_util; @@ -69,18 +75,18 @@ class ManagePasswordsBubbleViewTest : public ManagePasswordsTest { }; IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) { - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); ManagePasswordsBubbleView::ShowBubble( browser()->tab_strip_model()->GetActiveWebContents(), ManagePasswordsBubble::USER_ACTION); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); const ManagePasswordsBubbleView* bubble = ManagePasswordsBubbleView::manage_password_bubble(); EXPECT_TRUE(bubble->initially_focused_view()); EXPECT_EQ(bubble->initially_focused_view(), bubble->GetFocusManager()->GetFocusedView()); ManagePasswordsBubbleView::CloseBubble(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); // And, just for grins, ensure that we can re-open the bubble. ManagePasswordsBubbleView::ShowBubble( @@ -88,9 +94,9 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) { ManagePasswordsBubble::USER_ACTION); EXPECT_TRUE(ManagePasswordsBubbleView::manage_password_bubble()-> GetFocusManager()->GetFocusedView()); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); ManagePasswordsBubbleView::CloseBubble(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } // Same as 'BasicOpenAndClose', but use the command rather than the static @@ -98,30 +104,30 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) { IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandControlsBubble) { // The command only works if the icon is visible, so get into management mode. SetupManagingPasswords(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); ExecuteManagePasswordsCommand(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); const ManagePasswordsBubbleView* bubble = ManagePasswordsBubbleView::manage_password_bubble(); EXPECT_TRUE(bubble->initially_focused_view()); EXPECT_EQ(bubble->initially_focused_view(), bubble->GetFocusManager()->GetFocusedView()); ManagePasswordsBubbleView::CloseBubble(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); // And, just for grins, ensure that we can re-open the bubble. ExecuteManagePasswordsCommand(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); ManagePasswordsBubbleView::CloseBubble(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandExecutionInManagingState) { SetupManagingPasswords(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); ExecuteManagePasswordsCommand(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); scoped_ptr<base::HistogramSamples> samples( GetSamples(kDisplayDispositionMetric)); @@ -141,7 +147,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandExecutionInAutomaticState) { // Open with pending password: automagical! SetupPendingPassword(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); // Bubble should not be focused by default. EXPECT_FALSE(ManagePasswordsBubbleView::manage_password_bubble()-> @@ -168,11 +174,11 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandExecutionInPendingState) { // Open once with pending password: automagical! SetupPendingPassword(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); ManagePasswordsBubbleView::CloseBubble(); // This opening should be measured as manual. ExecuteManagePasswordsCommand(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); scoped_ptr<base::HistogramSamples> samples( GetSamples(kDisplayDispositionMetric)); @@ -191,12 +197,12 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandExecutionInAutomaticSaveState) { SetupAutomaticPassword(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); ManagePasswordsBubbleView::CloseBubble(); content::RunAllPendingInMessageLoop(); // Re-opening should count as manual. ExecuteManagePasswordsCommand(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); scoped_ptr<base::HistogramSamples> samples( GetSamples(kDisplayDispositionMetric)); @@ -216,11 +222,11 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CloseOnClick) { ManagePasswordsBubbleView::ShowBubble( browser()->tab_strip_model()->GetActiveWebContents(), ManagePasswordsBubble::AUTOMATIC); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); EXPECT_FALSE(ManagePasswordsBubbleView::manage_password_bubble()-> GetFocusManager()->GetFocusedView()); ui_test_utils::ClickOnView(browser(), VIEW_ID_TAB_CONTAINER); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CloseOnKey) { @@ -235,41 +241,41 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CloseOnKey) { browser()->tab_strip_model()->GetActiveWebContents(); ManagePasswordsBubbleView::ShowBubble(web_contents, ManagePasswordsBubble::AUTOMATIC); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); EXPECT_FALSE(ManagePasswordsBubbleView::manage_password_bubble()-> GetFocusManager()->GetFocusedView()); EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_TAB_CONTAINER)); EXPECT_TRUE(web_contents->GetRenderViewHost()->IsFocusedElementEditable()); ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_K, false, false, false, false)); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CloseOnChangedState) { SetupPendingPassword(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); // User navigated very fast and landed on another page with an autofilled // password. The save password bubble should disappear. SetupManagingPasswords(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, TwoTabsWithBubble) { // Set up the first tab with the bubble. SetupPendingPassword(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); // Set up the second tab. AddTabAtIndex(0, GURL("chrome://newtab"), ui::PAGE_TRANSITION_TYPED); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); ManagePasswordsBubbleView::ShowBubble( browser()->tab_strip_model()->GetActiveWebContents(), ManagePasswordsBubble::AUTOMATIC); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); TabStripModel* tab_model = browser()->tab_strip_model(); EXPECT_EQ(0, tab_model->active_index()); // Back to the first tab. tab_model->ActivateTabAt(1, true); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, ChooseCredential) { @@ -299,7 +305,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, ChooseCredential) { SetupChooseCredentials(local_credentials.Pass(), federated_credentials.Pass(), origin); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); EXPECT_CALL(*this, OnChooseCredential( Field(&password_manager::CredentialInfo::type, password_manager::CredentialType::CREDENTIAL_TYPE_EMPTY))); @@ -323,7 +329,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, ChooseCredentialNoFocus) { EXPECT_FALSE(browser()->window()->IsActive()); SetupChooseCredentials(local_credentials.Pass(), federated_credentials.Pass(), origin); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSignin) { @@ -349,11 +355,11 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSignin) { EXPECT_CALL(url_callback, OnRequestDone(avatar_url)); SetupAutoSignin(local_credentials.Pass()); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); ::testing::Mock::VerifyAndClearExpectations(&url_callback); ManagePasswordsBubbleView::CloseBubble(); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); content::RunAllPendingInMessageLoop(); // Open the bubble to manage accounts. @@ -362,7 +368,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSignin) { ManagePasswordsBubbleView::ShowBubble( browser()->tab_strip_model()->GetActiveWebContents(), ManagePasswordsBubble::USER_ACTION); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); } IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSigninNoFocus) { @@ -381,12 +387,12 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSigninNoFocus) { ManagePasswordsBubbleView::set_auto_signin_toast_timeout(0); SetupAutoSignin(local_credentials.Pass()); content::RunAllPendingInMessageLoop(); - EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_TRUE(IsBubbleShowing()); // Bring the first window back. The toast closes by timeout. focused_window->window()->Close(); browser()->window()->Activate(); content::RunAllPendingInMessageLoop(); EXPECT_TRUE(browser()->window()->IsActive()); - EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + EXPECT_FALSE(IsBubbleShowing()); } diff --git a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc index a0a54b1..2527a88 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc +++ b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc @@ -42,12 +42,6 @@ void ManagePasswordsIconView::OnChangingState() { ManagePasswordsBubbleView::CloseBubble(); } -bool ManagePasswordsIconView::IsBubbleShowing() const { - // If the bubble is being destroyed, it's considered showing though it may be - // already invisible currently. - return ManagePasswordsBubbleView::manage_password_bubble() != NULL; -} - void ManagePasswordsIconView::OnExecuting( BubbleIconView::ExecuteSource source) { } @@ -78,3 +72,7 @@ void ManagePasswordsIconView::AboutToRequestFocusFromTabTraversal( if (active()) ManagePasswordsBubbleView::ActivateBubble(); } + +views::BubbleDelegateView* ManagePasswordsIconView::GetBubble() const { + return ManagePasswordsBubbleView::manage_password_bubble(); +} diff --git a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h index 2d0c3d0..0b901c1 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h +++ b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h @@ -22,7 +22,6 @@ class ManagePasswordsIconView : public ManagePasswordsIcon, ~ManagePasswordsIconView() override; // BubbleIconView: - bool IsBubbleShowing() const override; void OnExecuting(BubbleIconView::ExecuteSource source) override; bool OnMousePressed(const ui::MouseEvent& event) override; bool OnKeyPressed(const ui::KeyEvent& event) override; @@ -40,6 +39,9 @@ class ManagePasswordsIconView : public ManagePasswordsIcon, void UpdateVisibleUI() override; void OnChangingState() override; + // BubbleIconView: + views::BubbleDelegateView* GetBubble() const override; + private: DISALLOW_COPY_AND_ASSIGN(ManagePasswordsIconView); diff --git a/chrome/browser/ui/views/tab_dialogs_views.cc b/chrome/browser/ui/views/tab_dialogs_views.cc index f235dce..517764d 100644 --- a/chrome/browser/ui/views/tab_dialogs_views.cc +++ b/chrome/browser/ui/views/tab_dialogs_views.cc @@ -53,7 +53,7 @@ void TabDialogsViews::ShowProfileSigninConfirmation( } void TabDialogsViews::ShowManagePasswordsBubble(bool user_action) { - if (ManagePasswordsBubbleView::IsShowing()) { + if (ManagePasswordsBubbleView::manage_password_bubble()) { // The bubble is currently shown for some other tab. We should close it now // and open for |web_contents_|. ManagePasswordsBubbleView::CloseBubble(); @@ -64,7 +64,7 @@ void TabDialogsViews::ShowManagePasswordsBubble(bool user_action) { } void TabDialogsViews::HideManagePasswordsBubble() { - if (!ManagePasswordsBubbleView::IsShowing()) + if (!ManagePasswordsBubbleView::manage_password_bubble()) return; content::WebContents* bubble_web_contents = ManagePasswordsBubbleView::manage_password_bubble()->web_contents(); diff --git a/chrome/browser/ui/views/translate/translate_bubble_view.cc b/chrome/browser/ui/views/translate/translate_bubble_view.cc index 145d444..d86cd49 100644 --- a/chrome/browser/ui/views/translate/translate_bubble_view.cc +++ b/chrome/browser/ui/views/translate/translate_bubble_view.cc @@ -85,7 +85,7 @@ void TranslateBubbleView::ShowBubble( translate::TranslateStep step, translate::TranslateErrors::Type error_type, bool is_user_gesture) { - if (IsShowing()) { + if (translate_bubble_view_) { // When the user reads the advanced setting panel, the bubble should not be // changed because he/she is focusing on the bubble. if (translate_bubble_view_->web_contents() == web_contents && @@ -133,18 +133,13 @@ void TranslateBubbleView::ShowBubble( // static void TranslateBubbleView::CloseBubble() { - if (!IsShowing()) + if (!translate_bubble_view_) return; translate_bubble_view_->GetWidget()->Close(); } // static -bool TranslateBubbleView::IsShowing() { - return translate_bubble_view_ != NULL; -} - -// static TranslateBubbleView* TranslateBubbleView::GetCurrentBubble() { return translate_bubble_view_; } diff --git a/chrome/browser/ui/views/translate/translate_bubble_view.h b/chrome/browser/ui/views/translate/translate_bubble_view.h index 014aa4d..61506e8 100644 --- a/chrome/browser/ui/views/translate/translate_bubble_view.h +++ b/chrome/browser/ui/views/translate/translate_bubble_view.h @@ -54,9 +54,6 @@ class TranslateBubbleView : public ManagedFullScreenBubbleDelegateView, // Closes the current bubble if existing. static void CloseBubble(); - // If true, the Translate bubble is being shown. - static bool IsShowing(); - // Returns the bubble view currently shown. This may return NULL. static TranslateBubbleView* GetCurrentBubble(); diff --git a/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc b/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc index fa856e4..1c0e41b 100644 --- a/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc +++ b/chrome/browser/ui/views/translate/translate_bubble_view_browsertest.cc @@ -40,7 +40,7 @@ class TranslateBubbleViewBrowserTest : public InProcessBrowserTest { // Flaky: crbug.com/394066 IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest, DISABLED_CloseBrowserWithoutTranslating) { - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); // Show a French page and wait until the bubble is shown. content::WebContents* current_web_contents = @@ -54,17 +54,17 @@ IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest, base::FilePath(), base::FilePath(FILE_PATH_LITERAL("french_page.html"))); ui_test_utils::NavigateToURL(browser(), french_url); fr_language_detected_signal.Wait(); - EXPECT_TRUE(TranslateBubbleView::IsShowing()); + EXPECT_TRUE(TranslateBubbleView::GetCurrentBubble()); // Close the window without translating. chrome::CloseWindow(browser()); - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); } // http://crbug.com/378061 IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest, DISABLED_CloseLastTabWithoutTranslating) { - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); // Show a French page and wait until the bubble is shown. content::WebContents* current_web_contents = @@ -78,17 +78,17 @@ IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest, base::FilePath(), base::FilePath(FILE_PATH_LITERAL("french_page.html"))); ui_test_utils::NavigateToURL(browser(), french_url); fr_language_detected_signal.Wait(); - EXPECT_TRUE(TranslateBubbleView::IsShowing()); + EXPECT_TRUE(TranslateBubbleView::GetCurrentBubble()); // Close the tab without translating. EXPECT_EQ(1, browser()->tab_strip_model()->count()); chrome::CloseWebContents(browser(), current_web_contents, false); - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); } IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest, CloseAnotherTabWithoutTranslating) { - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); int active_index = browser()->tab_strip_model()->active_index(); @@ -111,13 +111,13 @@ IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest, fr_language_detected_signal.Wait(); // The bubble is not shown because the tab is not activated. - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); // Close the French page tab immediately. chrome::CloseWebContents(browser(), web_contents, false); EXPECT_EQ(active_index, browser()->tab_strip_model()->active_index()); EXPECT_EQ(1, browser()->tab_strip_model()->count()); - EXPECT_FALSE(TranslateBubbleView::IsShowing()); + EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble()); // Close the last tab. chrome::CloseWebContents(browser(), |