diff options
author | pkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-22 06:55:31 +0000 |
---|---|---|
committer | pkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-22 06:55:31 +0000 |
commit | 82f67a43beca1d24d12fff48490645ae83225b76 (patch) | |
tree | 5c9ad28feaf28749fc1130e8618d12ff197636d0 | |
parent | a987515b634b4d8aaecc55ec4bb9491be8788b67 (diff) | |
download | chromium_src-82f67a43beca1d24d12fff48490645ae83225b76.zip chromium_src-82f67a43beca1d24d12fff48490645ae83225b76.tar.gz chromium_src-82f67a43beca1d24d12fff48490645ae83225b76.tar.bz2 |
This CL
- Anchors the zoom bubble to the magnifying glass when the zoom bubble is visible and the top-of-window view are revealed in immersive fullscreen. The top-of-window views stay revealed as long as the zoom bubble is visible.
- Positions the zoom bubble in the top right when in immersive fullscreen and the top-of-window views are not already revealed. For the sake of simplicity the zoom bubble is closed if the user reveals the top-of-window views.
BUG=181062
TEST=Manual, see bug
Review URL: https://chromiumcodereview.appspot.com/16998006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@208008 0039d316-1c4b-4281-b951-d872f2087c98
9 files changed, 291 insertions, 25 deletions
diff --git a/chrome/browser/ui/views/frame/immersive_mode_controller.cc b/chrome/browser/ui/views/frame/immersive_mode_controller.cc new file mode 100644 index 0000000..cebf6cb --- /dev/null +++ b/chrome/browser/ui/views/frame/immersive_mode_controller.cc @@ -0,0 +1,20 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/views/frame/immersive_mode_controller.h" + +ImmersiveModeController::ImmersiveModeController() { +} + +ImmersiveModeController::~ImmersiveModeController() { + FOR_EACH_OBSERVER(Observer, observers_, OnImmersiveModeControllerDestroyed()); +} + +void ImmersiveModeController::AddObserver(Observer* observer) { + observers_.AddObserver(observer); +} + +void ImmersiveModeController::RemoveObserver(Observer* observer) { + observers_.RemoveObserver(observer); +} diff --git a/chrome/browser/ui/views/frame/immersive_mode_controller.h b/chrome/browser/ui/views/frame/immersive_mode_controller.h index ae8a0eb..81da8d8 100644 --- a/chrome/browser/ui/views/frame/immersive_mode_controller.h +++ b/chrome/browser/ui/views/frame/immersive_mode_controller.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_UI_VIEWS_FRAME_IMMERSIVE_MODE_CONTROLLER_H_ #include "base/compiler_specific.h" +#include "base/observer_list.h" class BookmarkBarView; class FullscreenController; @@ -40,6 +41,18 @@ class ImmersiveModeController { ANIMATE_REVEAL_NO }; + class Observer { + public: + // Called when a reveal of the top-of-window views has been initiated. + virtual void OnImmersiveRevealStarted() {} + + // Called when the immersive mode controller has been destroyed. + virtual void OnImmersiveModeControllerDestroyed() {} + + protected: + virtual ~Observer() {} + }; + class Delegate { public: // Returns the bookmark bar, or NULL if the window does not support one. @@ -58,7 +71,8 @@ class ImmersiveModeController { virtual ~Delegate() {} }; - virtual ~ImmersiveModeController() {} + ImmersiveModeController(); + virtual ~ImmersiveModeController(); // Must initialize after browser view has a Widget and native window. virtual void Init(Delegate* delegate, @@ -127,6 +141,15 @@ class ImmersiveModeController { // visible. virtual void OnFindBarVisibleBoundsChanged( const gfx::Rect& new_visible_bounds_in_screen) = 0; + + virtual void AddObserver(Observer* observer); + virtual void RemoveObserver(Observer* observer); + + protected: + ObserverList<Observer> observers_; + + private: + DISALLOW_COPY_AND_ASSIGN(ImmersiveModeController); }; namespace chrome { diff --git a/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc index b7af186..7c31ac5 100644 --- a/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc +++ b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc @@ -288,11 +288,11 @@ ImmersiveModeControllerAsh::ImmersiveModeControllerAsh() revealed_lock_count_(0), tab_indicator_visibility_(TAB_INDICATORS_HIDE), mouse_x_when_hit_top_(-1), + gesture_begun_(false), native_window_(NULL), animation_(new ui::SlideAnimation(this)), animations_disabled_for_test_(false), - weak_ptr_factory_(this), - gesture_begun_(false) { + weak_ptr_factory_(this) { } ImmersiveModeControllerAsh::~ImmersiveModeControllerAsh() { @@ -944,8 +944,11 @@ void ImmersiveModeControllerAsh::MaybeStartReveal(Animate animate) { // Do not do any more processing if LayoutBrowserView() changed // |reveal_state_|. - if (reveal_state_ != SLIDING_OPEN) + if (reveal_state_ != SLIDING_OPEN) { + if (reveal_state_ == REVEALED) + FOR_EACH_OBSERVER(Observer, observers_, OnImmersiveRevealStarted()); return; + } } // Slide in the reveal view. if (animate == ANIMATE_NO) { @@ -955,6 +958,9 @@ void ImmersiveModeControllerAsh::MaybeStartReveal(Animate animate) { animation_->SetSlideDuration(GetAnimationDuration(animate)); animation_->Show(); } + + if (previous_reveal_state == CLOSED) + FOR_EACH_OBSERVER(Observer, observers_, OnImmersiveRevealStarted()); } void ImmersiveModeControllerAsh::EnablePaintToLayer(bool enable) { diff --git a/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h index c72c17d..09e3cda 100644 --- a/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h +++ b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h @@ -252,6 +252,10 @@ class ImmersiveModeControllerAsh : public ImmersiveModeController, // the top edge of the screen. int mouse_x_when_hit_top_; + // Tracks if the controller has seen a ET_GESTURE_SCROLL_BEGIN, without the + // following events. + bool gesture_begun_; + // The current visible bounds of the find bar, in screen coordinates. This is // an empty rect if the find bar is not visible. gfx::Rect find_bar_visible_bounds_in_screen_; @@ -284,10 +288,6 @@ class ImmersiveModeControllerAsh : public ImmersiveModeController, base::WeakPtrFactory<ImmersiveModeControllerAsh> weak_ptr_factory_; - // Tracks if the controller has seen a ET_GESTURE_SCROLL_BEGIN, without the - // following events. - bool gesture_begun_; - DISALLOW_COPY_AND_ASSIGN(ImmersiveModeControllerAsh); }; 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 7301f6d..76f1c7a 100644 --- a/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc +++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc @@ -46,9 +46,11 @@ void ZoomBubbleView::ShowBubble(content::WebContents* web_contents, DCHECK(browser && browser->window() && browser->fullscreen_controller()); BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser); - bool is_fullscreen = browser->window()->IsFullscreen(); - views::View* anchor_view = is_fullscreen ? - NULL : browser_view->GetLocationBarView()->zoom_view(); + bool is_fullscreen = browser_view->IsFullscreen(); + bool anchor_to_view = !is_fullscreen || + browser_view->immersive_mode_controller()->IsRevealed(); + views::View* anchor_view = anchor_to_view ? + browser_view->GetLocationBarView()->zoom_view() : NULL; // If the bubble is already showing in this window and its |auto_close_| value // is equal to |auto_close|, the bubble can be reused and only the label text @@ -66,11 +68,11 @@ void ZoomBubbleView::ShowBubble(content::WebContents* web_contents, zoom_bubble_ = new ZoomBubbleView(anchor_view, web_contents, auto_close, + browser_view->immersive_mode_controller(), browser->fullscreen_controller()); - // If we're fullscreen, there is no anchor view, so parent the bubble to - // the content area. - if (is_fullscreen) { + // If we do not have an anchor view, parent the bubble to the content area. + if (!anchor_to_view) { zoom_bubble_->set_parent_window( web_contents->GetView()->GetTopLevelNativeWindow()); } @@ -93,33 +95,58 @@ void ZoomBubbleView::CloseBubble() { // static bool ZoomBubbleView::IsShowing() { - return zoom_bubble_ != NULL; + // The bubble may be in the process of closing. + return zoom_bubble_ != NULL && zoom_bubble_->GetWidget()->IsVisible(); } -ZoomBubbleView::ZoomBubbleView(views::View* anchor_view, - content::WebContents* web_contents, - bool auto_close, - FullscreenController* fullscreen_controller) +// static +const ZoomBubbleView* ZoomBubbleView::GetZoomBubbleForTest() { + return zoom_bubble_; +} + +ZoomBubbleView::ZoomBubbleView( + views::View* anchor_view, + content::WebContents* web_contents, + bool auto_close, + ImmersiveModeController* immersive_mode_controller, + FullscreenController* fullscreen_controller) : BubbleDelegateView(anchor_view, anchor_view ? views::BubbleBorder::TOP_RIGHT : views::BubbleBorder::NONE), label_(NULL), web_contents_(web_contents), - auto_close_(auto_close) { + auto_close_(auto_close), + immersive_mode_controller_(immersive_mode_controller) { // Compensate for built-in vertical padding in the anchor view's image. set_anchor_view_insets(gfx::Insets(5, 0, 5, 0)); set_use_focusless(auto_close); set_notify_enter_exit_on_child(true); + if (anchor_view) { + // If we are in immersive fullscreen and the top-of-window views are + // already revealed, lock the top-of-window views in the revealed state + // as long as the zoom bubble is visible. ImmersiveModeController does + // not do this for us automatically because the zoom bubble is not + // activatable. + immersive_reveal_lock_.reset(immersive_mode_controller_->GetRevealedLock( + ImmersiveModeController::ANIMATE_REVEAL_NO)); + } + + // Add observers to close the bubble if the fullscreen state or immersive + // fullscreen revealed state changes. registrar_.Add(this, chrome::NOTIFICATION_FULLSCREEN_CHANGED, content::Source<FullscreenController>(fullscreen_controller)); + immersive_mode_controller_->AddObserver(this); } ZoomBubbleView::~ZoomBubbleView() { + if (immersive_mode_controller_) + immersive_mode_controller_->RemoveObserver(this); } void ZoomBubbleView::AdjustForFullscreen(const gfx::Rect& screen_bounds) { - DCHECK(!anchor_view()); + if (anchor_view()) + return; // TODO(dbeam): should RTL logic be done in views::BubbleDelegateView? const size_t bubble_half_width = width() / 2; @@ -218,6 +245,14 @@ void ZoomBubbleView::Observe(int type, CloseBubble(); } +void ZoomBubbleView::OnImmersiveRevealStarted() { + CloseBubble(); +} + +void ZoomBubbleView::OnImmersiveModeControllerDestroyed() { + immersive_mode_controller_ = NULL; +} + void ZoomBubbleView::WindowClosing() { // |zoom_bubble_| can be a new bubble by this point (as Close(); doesn't // call this right away). Only set to NULL when it's this 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 1bf4c8d..bb057ac 100644 --- a/chrome/browser/ui/views/location_bar/zoom_bubble_view.h +++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view.h @@ -7,6 +7,7 @@ #include "base/basictypes.h" #include "base/timer.h" +#include "chrome/browser/ui/views/frame/immersive_mode_controller.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "ui/views/bubble/bubble_delegate.h" @@ -24,7 +25,8 @@ class WebContents; // View used to display the zoom percentage when it has changed. class ZoomBubbleView : public views::BubbleDelegateView, public views::ButtonListener, - public content::NotificationObserver { + public content::NotificationObserver, + public ImmersiveModeController::Observer { public: // Shows the bubble and automatically closes it after a short time period if // |auto_close| is true. @@ -37,16 +39,22 @@ class ZoomBubbleView : public views::BubbleDelegateView, // 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(); + private: ZoomBubbleView(views::View* anchor_view, content::WebContents* web_contents, bool auto_close, + ImmersiveModeController* immersive_mode_controller, FullscreenController* fullscreen_controller); virtual ~ZoomBubbleView(); - // Place the bubble in the top right (left in RTL) of the |screen_bounds| that - // contain |web_contents_|'s browser window. Because the positioning is based - // on the size of the bubble, this must be called after the bubble is created. + // If the bubble is not anchored to a view, places the bubble in the top + // right (left in RTL) of the |screen_bounds| that contain |web_contents_|'s + // browser window. Because the positioning is based on the size of the + // bubble, this must be called after the bubble is created. void AdjustForFullscreen(const gfx::Rect& screen_bounds); // Refreshes the bubble by changing the zoom percentage appropriately and @@ -81,6 +89,10 @@ class ZoomBubbleView : public views::BubbleDelegateView, const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; + // ImmersiveModeController::Observer methods. + virtual void OnImmersiveRevealStarted() OVERRIDE; + virtual void OnImmersiveModeControllerDestroyed() OVERRIDE; + // Singleton instance of the zoom bubble. The zoom bubble can only be shown on // the active browser window, so there is no case in which it will be shown // twice at the same time. @@ -98,6 +110,15 @@ class ZoomBubbleView : public views::BubbleDelegateView, // Whether the currently displayed bubble will automatically close. bool auto_close_; + // The immersive mode controller for the BrowserView containing + // |web_contents_|. + // Not owned. + ImmersiveModeController* immersive_mode_controller_; + + // Keeps the top-of-window views revealed (but does not initiate a reveal) + // when the bubble is visible in immersive fullscreen. + scoped_ptr<ImmersiveRevealedLock> immersive_reveal_lock_; + // Used to register for fullscreen change notifications. content::NotificationRegistrar registrar_; 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 new file mode 100644 index 0000000..bdc76a7 --- /dev/null +++ b/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc @@ -0,0 +1,159 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/views/location_bar/zoom_bubble_view.h" + +#include "chrome/browser/ui/browser_commands.h" +#include "chrome/browser/ui/fullscreen/fullscreen_controller.h" +#include "chrome/browser/ui/fullscreen/fullscreen_controller_test.h" +#include "chrome/browser/ui/immersive_fullscreen_configuration.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/frame/immersive_mode_controller.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" + +#if defined(OS_CHROMEOS) +#include "chrome/browser/ui/views/frame/immersive_mode_controller_ash.h" +#endif + +class ZoomBubbleBrowserTest : public InProcessBrowserTest { + public: + ZoomBubbleBrowserTest() {} + virtual ~ZoomBubbleBrowserTest() {} + + virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE { + ImmersiveFullscreenConfiguration::EnableImmersiveFullscreenForTest(); + } + + private: + DISALLOW_COPY_AND_ASSIGN(ZoomBubbleBrowserTest); +}; + +// TODO(linux_aura) http://crbug.com/163931 +#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_AURA) +#define MAYBE_NonImmersiveFullscreen DISABLED_NonImmersiveFullscreen +#else +#define MAYBE_NonImmersiveFullscreen NonImmersiveFullscreen +#endif +// Test whether the zoom bubble is anchored and whether it is visible when in +// non-immersive fullscreen. +IN_PROC_BROWSER_TEST_F(ZoomBubbleBrowserTest, MAYBE_NonImmersiveFullscreen) { + BrowserView* browser_view = static_cast<BrowserView*>(browser()->window()); + content::WebContents* web_contents = browser_view->GetActiveWebContents(); + + // The zoom bubble should be anchored when not in fullscreen. + ZoomBubbleView::ShowBubble(web_contents, false); + ASSERT_TRUE(ZoomBubbleView::IsShowing()); + const ZoomBubbleView* zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + EXPECT_TRUE(zoom_bubble->anchor_view()); + +#if defined(OS_CHROMEOS) + // Switching to tab fullscreen while having the zoom bubble open triggers a + // DCHECK in WorkspaceManager::ReparentWindow() during the test but not in + // when running chrome. + // TODO(pkotwicz): Investigate DCHECK. + ZoomBubbleView::CloseBubble(); + // The zoom bubble is deleted on a task. + content::RunAllPendingInMessageLoop(); +#endif + + // Entering fullscreen should close the bubble. (We enter into tab fullscreen + // here because tab fullscreen is non-immersive even when + // ImmersiveFullscreenConfiguration::UseImmersiveFullscreen()) returns + // true. + { + // NOTIFICATION_FULLSCREEN_CHANGED is sent asynchronously. Wait for the + // notification before testing the zoom bubble visibility. + scoped_ptr<FullscreenNotificationObserver> waiter( + new FullscreenNotificationObserver()); + browser()->fullscreen_controller()->ToggleFullscreenModeForTab( + web_contents, true); + waiter->Wait(); + } + ASSERT_FALSE(browser_view->immersive_mode_controller()->IsEnabled()); + EXPECT_FALSE(ZoomBubbleView::IsShowing()); + + // The bubble should not be anchored when it is shown in non-immersive + // fullscreen. + ZoomBubbleView::ShowBubble(web_contents, false); + ASSERT_TRUE(ZoomBubbleView::IsShowing()); + zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + EXPECT_FALSE(zoom_bubble->anchor_view()); + + // Exit fullscreen before ending the test for the sake of sanity. + { + scoped_ptr<FullscreenNotificationObserver> waiter( + new FullscreenNotificationObserver()); + chrome::ToggleFullscreenMode(browser()); + waiter->Wait(); + } +} + +// Immersive fullscreen is CrOS only for now. +#if defined(OS_CHROMEOS) +// Test whether the zoom bubble is anchored and whether it is visible when in +// immersive fullscreen. +IN_PROC_BROWSER_TEST_F(ZoomBubbleBrowserTest, ImmersiveFullscreen) { + BrowserView* browser_view = static_cast<BrowserView*>(browser()->window()); + content::WebContents* web_contents = browser_view->GetActiveWebContents(); + + ASSERT_TRUE(ImmersiveFullscreenConfiguration::UseImmersiveFullscreen()); + ImmersiveModeControllerAsh* immersive_controller = + static_cast<ImmersiveModeControllerAsh*>( + browser_view->immersive_mode_controller()); + immersive_controller->DisableAnimationsForTest(); + + // Move the mouse out of the way. + immersive_controller->SetMouseHoveredForTest(false); + + // Enter immersive fullscreen. + { + scoped_ptr<FullscreenNotificationObserver> waiter( + new FullscreenNotificationObserver()); + chrome::ToggleFullscreenMode(browser()); + waiter->Wait(); + } + ASSERT_TRUE(immersive_controller->IsEnabled()); + ASSERT_FALSE(immersive_controller->IsRevealed()); + + // 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, false); + ASSERT_TRUE(ZoomBubbleView::IsShowing()); + const ZoomBubbleView* zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + EXPECT_FALSE(zoom_bubble->anchor_view()); + + // An immersive reveal should hide the zoom bubble. + scoped_ptr<ImmersiveRevealedLock> immersive_reveal_lock( + immersive_controller->GetRevealedLock( + ImmersiveModeController::ANIMATE_REVEAL_NO)); + ASSERT_TRUE(immersive_controller->IsRevealed()); + EXPECT_FALSE(ZoomBubbleView::IsShowing()); + + // 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, false); + ASSERT_TRUE(ZoomBubbleView::IsShowing()); + zoom_bubble = ZoomBubbleView::GetZoomBubbleForTest(); + EXPECT_TRUE(zoom_bubble->anchor_view()); + + // The top-of-window views should not hide till the zoom bubble hides. (It + // would be weird if the view to which the zoom bubble is anchored hid while + // the zoom bubble was still visible.) + immersive_reveal_lock.reset(); + EXPECT_TRUE(immersive_controller->IsRevealed()); + ZoomBubbleView::CloseBubble(); + // The zoom bubble is deleted on a task. + content::RunAllPendingInMessageLoop(); + EXPECT_FALSE(immersive_controller->IsRevealed()); + + // Exit fullscreen before ending the test for the sake of sanity. + { + scoped_ptr<FullscreenNotificationObserver> waiter( + new FullscreenNotificationObserver()); + chrome::ToggleFullscreenMode(browser()); + waiter->Wait(); + } +} +#endif // OS_CHROMEOS diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index 9a0da04..461c45b 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -1690,6 +1690,7 @@ 'browser/ui/views/frame/browser_desktop_root_window_host_x11.h', 'browser/ui/views/frame/glass_browser_frame_view.cc', 'browser/ui/views/frame/glass_browser_frame_view.h', + 'browser/ui/views/frame/immersive_mode_controller.cc', 'browser/ui/views/frame/immersive_mode_controller.h', 'browser/ui/views/frame/immersive_mode_controller_ash.cc', 'browser/ui/views/frame/immersive_mode_controller_ash.h', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 4155c7d..987004d 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1614,6 +1614,7 @@ 'browser/ui/views/frame/app_non_client_frame_view_ash_browsertest.cc', 'browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc', 'browser/ui/views/frame/immersive_mode_controller_ash_browsertest.cc', + 'browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc', 'browser/ui/views/select_file_dialog_extension_browsertest.cc', 'browser/ui/views/sync/one_click_signin_bubble_view_browsertest.cc', 'browser/ui/views/toolbar_view_browsertest.cc', |