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

chrome/browser/ui/views/frame/immersive_mode_controller.cc

+// 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);
+}

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 {

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) {

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);
 };
 

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.

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_;
 

chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc

+// 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

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',

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',