Fix bugs wrt focus and anchored widgets and immersive mode being disabled. See...

Fix bugs wrt focus and anchored widgets and immersive mode being disabled. See bugs below for more details.

BUG=188567, 233483
Test=ImmersiveModeControllerAshTest.*

Review URL: https://chromiumcodereview.appspot.com/13942011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195364 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -122,9 +122,12 @@ class ImmersiveModeControllerAsh::AnchoredWidgetManager
   // keeping the top-of-window views revealed.
   void RemoveAnchoredWidget(views::Widget* widget);
 
-  // Adjusts the y positions of the anchored widgets for the new top container
-  // bounds.
-  void OnTopContainerBoundsChanged();
+  // Repositions the anchored widgets for the current top container bounds if
+  // immersive mode is enabled.
+  void MaybeRepositionAnchoredWidgets();
+
+  // Called when immersive mode has been enabled.
+  void OnImmersiveModeEnabled();
 
   const std::set<views::Widget*>& visible_anchored_widgets() const {
     return visible_;
@@ -203,7 +206,7 @@ void ImmersiveModeControllerAsh::AnchoredWidgetManager::RemoveAnchoredWidget(
 }
 
 void ImmersiveModeControllerAsh::AnchoredWidgetManager::
-    OnTopContainerBoundsChanged() {
+    MaybeRepositionAnchoredWidgets() {
   for (std::map<views::Widget*, int>::iterator it = widgets_.begin();
        it != widgets_.end(); ++it) {
     UpdateWidgetBounds(it->first, it->second);
@@ -212,6 +215,14 @@ void ImmersiveModeControllerAsh::AnchoredWidgetManager::
   UpdateRevealedLock();
 }
 
+void ImmersiveModeControllerAsh::AnchoredWidgetManager::
+    OnImmersiveModeEnabled() {
+  UpdateRevealedLock();
+  // The top container bounds may have changed while immersive mode was
+  // disabled.
+  MaybeRepositionAnchoredWidgets();
+}
+
 void ImmersiveModeControllerAsh::AnchoredWidgetManager::UpdateRevealedLock() {
   if (visible_.empty()) {
     revealed_lock_.reset();
@@ -238,7 +249,7 @@ void ImmersiveModeControllerAsh::AnchoredWidgetManager::UpdateRevealedLock() {
 void ImmersiveModeControllerAsh::AnchoredWidgetManager::UpdateWidgetBounds(
     views::Widget* widget,
     int y_offset) {
-  if (!widget->IsVisible())
+  if (!controller_->IsEnabled() || !widget->IsVisible())
     return;
 
   gfx::Rect top_container_target_bounds =
@@ -382,22 +393,19 @@ void ImmersiveModeControllerAsh::SetEnabled(bool enabled) {
     MaybeStartReveal(ANIMATE_NO);
 
     // Reset the mouse and the focus revealed locks so that they do not affect
-    // whether the top-of-window views are hidden. Reacquire the locks if ending
-    // the reveal is unsuccessful.
-    bool had_mouse_revealed_lock = (mouse_revealed_lock_.get() != NULL);
-    bool had_focus_revealed_lock = (focus_revealed_lock_.get() != NULL);
+    // whether the top-of-window views are hidden.
     mouse_revealed_lock_.reset();
     focus_revealed_lock_.reset();
 
     // Try doing the animation.
     MaybeEndReveal(ANIMATE_SLOW);
 
-    if (IsRevealed()) {
-      if (had_mouse_revealed_lock)
-        mouse_revealed_lock_.reset(GetRevealedLock());
-      if (had_focus_revealed_lock)
-        focus_revealed_lock_.reset(GetRevealedLock());
+    if (reveal_state_ == REVEALED) {
+      // Reveal was unsuccessful. Reacquire the revealed locks if appropriate.
+      UpdateMouseRevealedLock(true);
+      UpdateFocusRevealedLock();
     }
+    anchored_widget_manager_->OnImmersiveModeEnabled();
   } else {
     // Stop cursor-at-top tracking.
     top_timer_.Stop();
@@ -461,7 +469,7 @@ void ImmersiveModeControllerAsh::UnanchorWidgetFromTopContainer(
 }
 
 void ImmersiveModeControllerAsh::OnTopContainerBoundsChanged() {
-  anchored_widget_manager_->OnTopContainerBoundsChanged();
+  anchored_widget_manager_->MaybeRepositionAnchoredWidgets();
 }
 
 ////////////////////////////////////////////////////////////////////////////////

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

@@ -10,6 +10,7 @@
 #include "ash/shelf/shelf_types.h"
 #include "ash/shell.h"
 #include "ash/wm/window_properties.h"
+#include "ash/wm/window_util.h"
 #include "base/command_line.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
@@ -269,7 +270,33 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshTest, Focus) {
   browser_view->GetTabContentsContainerView()->RequestFocus();
   EXPECT_FALSE(controller->IsRevealed());
 
-  // 5) Test that a dialog opened by the web contents does not initiate a
+  // 5) Test that a loss of focus of the location bar to the web contents
+  // while immersive mode is disabled is properly registered.
+  browser_view->SetFocusToLocationBar(false);
+  EXPECT_TRUE(controller->IsRevealed());
+  chrome::ToggleFullscreenMode(browser());
+  EXPECT_FALSE(controller->IsEnabled());
+  EXPECT_FALSE(controller->IsRevealed());
+  browser_view->GetTabContentsContainerView()->RequestFocus();
+  chrome::ToggleFullscreenMode(browser());
+  EXPECT_TRUE(controller->IsEnabled());
+  EXPECT_FALSE(controller->IsRevealed());
+
+  // Repeat test but with a revealed lock acquired when immersive mode is
+  // enabled because the code path is different.
+  browser_view->SetFocusToLocationBar(false);
+  EXPECT_TRUE(controller->IsRevealed());
+  chrome::ToggleFullscreenMode(browser());
+  scoped_ptr<ImmersiveModeController::RevealedLock> lock(
+      controller->GetRevealedLock());
+  EXPECT_FALSE(controller->IsRevealed());
+  browser_view->GetTabContentsContainerView()->RequestFocus();
+  chrome::ToggleFullscreenMode(browser());
+  EXPECT_TRUE(controller->IsRevealed());
+  lock.reset();
+  EXPECT_FALSE(controller->IsRevealed());
+
+  // 6) Test that a dialog opened by the web contents does not initiate a
   // reveal.
   AppModalDialogQueue* queue = AppModalDialogQueue::GetInstance();
   EXPECT_FALSE(queue->HasActiveDialog());
@@ -427,6 +454,7 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshTest, AnchoredWidgets) {
 
   views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
   params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.parent = browser_view->GetNativeWindow();
   params.bounds = kInitialBounds;
   views::Widget anchored_widget;
   anchored_widget.Init(params);
@@ -504,6 +532,33 @@ IN_PROC_BROWSER_TEST_F(ImmersiveModeControllerAshTest, AnchoredWidgets) {
   EXPECT_EQ(kInitialBounds.x(), bounds3.x());
   EXPECT_EQ(kInitialBounds.size(), bounds3.size());
 
+  // 3) Test that the anchored widget is not repositioned when immersive mode
+  // is not enabled.
+  chrome::ToggleFullscreenMode(browser());
+  ASSERT_FALSE(controller->IsEnabled());
+  chrome::ExecuteCommand(browser(), IDC_SHOW_BOOKMARK_BAR);
+  EXPECT_EQ(bounds3, anchored_widget.GetWindowBoundsInScreen());
+  EXPECT_NE(browser_view->top_container()->GetBoundsInScreen().bottom() + 10,
+            bounds3.bottom());
+
+  // 4) Test that reenabling immersive fullscreen repositions any anchored
+  // widgets.
+  //
+  // Maximize the window so that a bounds change in the top container is not
+  // reported when entering immersive fullscreen. The top container has the
+  // same bounds when |browser_view| is maximized as when the top container is
+  // revealed when |browser_view_| is in immersive mode.
+  ash::wm::MaximizeWindow(browser_view->GetNativeWindow());
+  chrome::ToggleFullscreenMode(browser());
+  ASSERT_TRUE(controller->IsEnabled());
+
+  gfx::Rect bounds4 = anchored_widget.GetWindowBoundsInScreen();
+  EXPECT_NE(bounds3, bounds4);
+  EXPECT_EQ(bounds4.y(),
+            browser_view->top_container()->GetBoundsInScreen().bottom() + 10);
+  EXPECT_EQ(kInitialBounds.x(), bounds4.x());
+  EXPECT_EQ(kInitialBounds.size(), bounds4.size());
+
   BookmarkBarView::DisableAnimationsForTesting(false);
 }