Windowed mode mouse lock addded to fullscreen controller.

chrome/browser/ui/fullscreen_controller.cc

 // Copyright (c) 2012 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/fullscreen_controller.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
@@ skipping 16 matching lines @@
 
 FullscreenController::FullscreenController(BrowserWindow* window,
                                            Profile* profile,
                                            Browser* browser)
   : window_(window),
     profile_(profile),
     browser_(browser),
     fullscreened_tab_(NULL),
     tab_caused_fullscreen_(false),
     tab_fullscreen_accepted_(false),
+    in_or_entering_fullscreen_(false),
+    mouse_lock_tab_(NULL),
     mouse_lock_state_(MOUSELOCK_NOT_REQUESTED) {
 }
 
 bool FullscreenController::IsFullscreenForBrowser() const {
   return window_->IsFullscreen() && !tab_caused_fullscreen_;
 }
 
 bool FullscreenController::IsFullscreenForTabOrPending() const {
   return fullscreened_tab_ != NULL;
 }
@@ skipping 10 matching lines @@
 
 bool FullscreenController::IsMouseLockRequested() const {
   return mouse_lock_state_ == MOUSELOCK_REQUESTED;
 }
 
 bool FullscreenController::IsMouseLocked() const {
   return mouse_lock_state_ == MOUSELOCK_ACCEPTED;
 }
 
 void FullscreenController::RequestToLockMouse(WebContents* tab,
-                                              bool /* user_gesture */) {
-  // TODO(scheib) user_gesture required for Mouse Lock in Windowed Mode.
-  // See http://crbug.com/107013, which will land in multiple patches.
+                                              bool user_gesture) {
+  DCHECK(!IsMouseLocked());
+  MessageLoop::current()->PostTask(FROM_HERE,
+      base::Bind(&FullscreenController::NotifyMouseLockChange, this));

[yzshen1, 2012/04/30 19:32:40]

Is it sufficient to send chrome::NOTIFICATION_MOUSE_LOCK_CHANGED when
mouse_lock_state_ changes?

If that is true, the code can be simplified a great deal, and it is more clear
when such a notification gets sent.
(You can replace mouse_lock_state_ = ... statements with SetMouseLockState() and
send notification in it.)

[scheib, 2012/05/01 03:27:07]

Tests are the reason this notification exists. A test that requests mouse lock
without a user gesture needs to know that the request has been handled and a
decision made. A callback is made to JavaScript, but I didn't see an easy way to
have a browser test block until a JS callback is made. The notification here is
very easy for test code to wait on, but needs to fire in all decision events
(not only a state change).

 
-  DCHECK(!IsMouseLocked());
-
-  // Mouse Lock is only permitted when browser is in tab fullscreen.
-  if (!IsFullscreenForTabOrPending(tab)) {
+  // Must have a user gesture, or we must already be in tab fullscreen.
+  if (!user_gesture && !IsFullscreenForTabOrPending(tab)) {
     tab->GotResponseToLockMouseRequest(false);
     return;
   }
 
+  mouse_lock_tab_ = TabContentsWrapper::GetCurrentWrapperForContents(tab);
+  FullscreenExitBubbleType bubble_type = GetFullscreenExitBubbleType();
+
   switch (GetMouseLockSetting(tab->GetURL())) {
     case CONTENT_SETTING_ALLOW:
-      if (tab_fullscreen_accepted_) {
-        if (tab->GotResponseToLockMouseRequest(true))
-          mouse_lock_state_ = MOUSELOCK_ACCEPTED;
-      } else {
+      // If bubble already displaying buttons, merge mouse lock request.
+      if (fullscreen_bubble::ShowButtonsForType(bubble_type)) {
         mouse_lock_state_ = MOUSELOCK_REQUESTED;

[yzshen1, 2012/04/30 19:32:40]

Please add more comment about why we need to merge (click is required so we
cannot go into mouselock, etc.)

[scheib, 2012/05/01 03:27:07]

Done.

+        break;

[yzshen1, 2012/04/30 19:32:40]

You could use an "else" on line 89 instead of two breaks in this case.

[scheib, 2012/05/01 03:27:07]

Done.

       }
+
+      // Lock mouse.
+      if (tab->GotResponseToLockMouseRequest(true))
+        mouse_lock_state_ = MOUSELOCK_ACCEPTED;
+      else
+        mouse_lock_tab_ = NULL;

[yzshen1, 2012/04/30 19:32:40]

Shall we explicitly set mouse_lock_state_ to not requested in this case?

[scheib, 2012/05/01 03:27:07]

Done.

       break;
     case CONTENT_SETTING_BLOCK:
       tab->GotResponseToLockMouseRequest(false);
+      mouse_lock_tab_ = NULL;

[yzshen1, 2012/04/30 19:32:40]

Shall we set mouse_lock_state_ to not requested in this case?

[scheib, 2012/05/01 03:27:07]

Done.

       break;
     case CONTENT_SETTING_ASK:
       mouse_lock_state_ = MOUSELOCK_REQUESTED;
       break;
     default:
       NOTREACHED();
   }
   UpdateFullscreenExitBubbleContent();
 }
 
@@ skipping 35 matching lines @@
         TogglePresentationModeInternal(true);
 #else
         ToggleFullscreenModeInternal(true);
 #endif
       } else {
         // If currently there is a tab in "tab fullscreen" mode and fullscreen
         // was not caused by it (i.e., previously it was in "browser fullscreen"
         // mode), we need to switch back to "browser fullscreen" mode. In this
         // case, all we have to do is notifying the tab that it has exited "tab
         // fullscreen" mode.
-        NotifyTabOfFullscreenExitIfNecessary();
+        NotifyTabOfExitIfNecessary();
       }
     }
   }
 }
 
 #if defined(OS_MACOSX)
 void FullscreenController::TogglePresentationMode() {
   TogglePresentationModeInternal(false);
 }
 #endif
 
 void FullscreenController::ToggleFullscreenMode() {
   extension_caused_fullscreen_ = GURL();
   ToggleFullscreenModeInternal(false);
 }
 
 void FullscreenController::ToggleFullscreenModeWithExtension(
     const GURL& extension_url) {
   // |extension_caused_fullscreen_| will be reset if this causes fullscreen to
   // exit.
   extension_caused_fullscreen_ = extension_url;
   ToggleFullscreenModeInternal(false);
 }
 
 void FullscreenController::LostMouseLock() {
   mouse_lock_state_ = MOUSELOCK_NOT_REQUESTED;
+  MessageLoop::current()->PostTask(FROM_HERE,
+      base::Bind(&FullscreenController::NotifyMouseLockChange, this));
   UpdateFullscreenExitBubbleContent();
 }
 
 void FullscreenController::OnTabClosing(WebContents* web_contents) {
   if (IsFullscreenForTabOrPending(web_contents)) {
     ExitTabbedFullscreenModeIfNecessary();
     // The call to exit fullscreen may result in asynchronous notification of
     // fullscreen state change (e.g., on Linux). We don't want to rely on it
-    // to call NotifyTabOfFullscreenExitIfNecessary(), because at that point
+    // to call NotifyTabOfExitIfNecessary(), because at that point
     // |fullscreen_tab_| may not be valid. Instead, we call it here to clean up
     // tab fullscreen related state.
-    NotifyTabOfFullscreenExitIfNecessary();
+    NotifyTabOfExitIfNecessary();
   }
 }
 
 void FullscreenController::OnTabDeactivated(TabContentsWrapper* contents) {
   if (contents == fullscreened_tab_)
     ExitTabbedFullscreenModeIfNecessary();
 }
 
 void FullscreenController::OnAcceptFullscreenPermission(
     const GURL& url,
     FullscreenExitBubbleType bubble_type) {
   bool mouse_lock = false;
   bool fullscreen = false;
   fullscreen_bubble::PermissionRequestedByType(bubble_type, &fullscreen,
                                                &mouse_lock);
-  DCHECK(fullscreened_tab_);
-  DCHECK_NE(tab_fullscreen_accepted_, fullscreen);
+  DCHECK(!(fullscreen && tab_fullscreen_accepted_));
+  DCHECK(!(mouse_lock && IsMouseLocked()));
 
   HostContentSettingsMap* settings_map = profile_->GetHostContentSettingsMap();
   ContentSettingsPattern pattern = ContentSettingsPattern::FromURL(url);
-  if (mouse_lock) {
+
+  if (mouse_lock && !IsMouseLocked()) {
     DCHECK(IsMouseLockRequested());
     // TODO(markusheintz): We should allow patterns for all possible URLs here.
     if (pattern.IsValid()) {
       settings_map->SetContentSetting(
           pattern, ContentSettingsPattern::Wildcard(),
           CONTENT_SETTINGS_TYPE_MOUSELOCK, std::string(),
           CONTENT_SETTING_ALLOW);
     }
-    mouse_lock_state_ =
-        fullscreened_tab_->web_contents()->GotResponseToLockMouseRequest(true) ?
-        MOUSELOCK_ACCEPTED : MOUSELOCK_NOT_REQUESTED;
+
+    if (mouse_lock_tab_ &&
+        mouse_lock_tab_->web_contents() &&
+        mouse_lock_tab_->web_contents()->GotResponseToLockMouseRequest(true)) {
+        mouse_lock_state_ = MOUSELOCK_ACCEPTED;
+    } else {
+      mouse_lock_state_ = MOUSELOCK_NOT_REQUESTED;
+      mouse_lock_tab_ = NULL;
+    }
+
+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&FullscreenController::NotifyMouseLockChange, this));
   }
-  if (!tab_fullscreen_accepted_) {
+
+  if (fullscreen && !tab_fullscreen_accepted_) {
+    DCHECK(fullscreened_tab_);
     if (pattern.IsValid()) {
       settings_map->SetContentSetting(
           pattern, ContentSettingsPattern::Wildcard(),
           CONTENT_SETTINGS_TYPE_FULLSCREEN, std::string(),
           CONTENT_SETTING_ALLOW);
     }
     tab_fullscreen_accepted_ = true;
   }
   UpdateFullscreenExitBubbleContent();
 }
 
 void FullscreenController::OnDenyFullscreenPermission(
     FullscreenExitBubbleType bubble_type) {
   bool mouse_lock = false;
   bool fullscreen = false;
   fullscreen_bubble::PermissionRequestedByType(bubble_type, &fullscreen,
                                                &mouse_lock);
-  DCHECK(fullscreened_tab_);
-  DCHECK_NE(tab_fullscreen_accepted_, fullscreen);
+  DCHECK(fullscreened_tab_ || mouse_lock_tab_);
+  DCHECK(!(fullscreen && tab_fullscreen_accepted_));
+  DCHECK(!(mouse_lock && IsMouseLocked()));
 
   if (mouse_lock) {
     DCHECK(IsMouseLockRequested());
     mouse_lock_state_ = MOUSELOCK_NOT_REQUESTED;
-    fullscreened_tab_->web_contents()->GotResponseToLockMouseRequest(false);
-    if (!fullscreen)
-      UpdateFullscreenExitBubbleContent();
+    if (mouse_lock_tab_ && mouse_lock_tab_->web_contents())
+      mouse_lock_tab_->web_contents()->GotResponseToLockMouseRequest(false);
+    mouse_lock_tab_ = NULL;
+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&FullscreenController::NotifyMouseLockChange, this));
   }
 
   if (fullscreen)
     ExitTabbedFullscreenModeIfNecessary();
+
+  UpdateFullscreenExitBubbleContent();

[yzshen1, 2012/04/30 19:32:40]

nit: It is previously in the "if (mouse_lock)" block because
ExitTabbedFullscreenModeIfNecessary() will update bubble content if necessary.

[scheib, 2012/05/01 03:27:07]

Done. -- Reverted to previous code; added comment explaining why.

 }
 
 void FullscreenController::WindowFullscreenStateChanged() {
   MessageLoop::current()->PostTask(FROM_HERE,
       base::Bind(&FullscreenController::NotifyFullscreenChange, this));
   bool exiting_fullscreen;
 #if defined(OS_MACOSX)
   exiting_fullscreen = !window_->InPresentationMode();
 #else
   exiting_fullscreen = !window_->IsFullscreen();
 #endif
   if (exiting_fullscreen)
-    NotifyTabOfFullscreenExitIfNecessary();
+    NotifyTabOfExitIfNecessary();
 #if !defined(OS_CHROMEOS)
   if (exiting_fullscreen)
     window_->GetDownloadShelf()->Unhide();
   else
     window_->GetDownloadShelf()->Hide();
 #endif
 }
 
 bool FullscreenController::HandleUserPressedEscape() {
-  if (!IsFullscreenForTabOrPending())
-    return false;
-  ExitTabbedFullscreenModeIfNecessary();
-  return true;
+  if (IsFullscreenForTabOrPending() ||
+      IsMouseLocked() || IsMouseLockRequested()) {
+    ExitTabbedFullscreenModeIfNecessary();

[yzshen1, 2012/04/30 19:32:40]

I don't get it: in windowed mouse lock mode, why do we need to
ExitTabbedFullscreenModeIfNecessary()? Maybe we should change the name/meaning
of the method?

[scheib, 2012/05/01 03:27:07]

Done: Renamed to ExitTabFullscreenOrMouseLockIfNecessary().

+    return true;
+  }
+
+  return false;
 }
 
 FullscreenController::~FullscreenController() {}
 
-void FullscreenController::NotifyTabOfFullscreenExitIfNecessary() {
+void FullscreenController::NotifyTabOfExitIfNecessary() {
   if (fullscreened_tab_) {
     RenderViewHost* rvh =
         fullscreened_tab_->web_contents()->GetRenderViewHost();
     fullscreened_tab_ = NULL;
     tab_caused_fullscreen_ = false;
     tab_fullscreen_accepted_ = false;
-    mouse_lock_state_ = MOUSELOCK_NOT_REQUESTED;
     if (rvh)
       rvh->ExitFullscreen();
-  } else {
-    DCHECK_EQ(mouse_lock_state_, MOUSELOCK_NOT_REQUESTED);
+  }
+
+  if (mouse_lock_tab_) {
+    WebContents* web_contents = mouse_lock_tab_->web_contents();
+    mouse_lock_tab_ = NULL;
+    mouse_lock_state_ = MOUSELOCK_NOT_REQUESTED;
+    if (web_contents)
+      web_contents->GotResponseToLockMouseRequest(false);

[yzshen1, 2012/04/30 19:32:40]

If in ACCEPTED mode, we shouldn't call GotResponseToLockMouseRequest, right?

[scheib, 2012/05/01 03:27:07]

Done. Changed to calling RVHV::UnlockMouse() when we aren't PENDING.

+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&FullscreenController::NotifyMouseLockChange, this));
   }
 
   UpdateFullscreenExitBubbleContent();
 }
 
 void FullscreenController::ExitTabbedFullscreenModeIfNecessary() {
   if (tab_caused_fullscreen_)
     ToggleFullscreenMode();
   else
-    NotifyTabOfFullscreenExitIfNecessary();
+    NotifyTabOfExitIfNecessary();
 }
 
 void FullscreenController::UpdateFullscreenExitBubbleContent() {
   GURL url;
   if (fullscreened_tab_)
     url = fullscreened_tab_->web_contents()->GetURL();
   else if (!extension_caused_fullscreen_.is_empty())
     url = extension_caused_fullscreen_;
+  else if (mouse_lock_tab_)

[yzshen1, 2012/04/30 19:32:40]

Is this order right?
Consider the following two sequences:
1) enter extension-triggered fullscreen -> enter mouse lock
2) enter mouse lock -> enter extension-triggered fullscreen

in these two cases, which URL shall we show? the URL of the extension or the
mouse lock tab?

[scheib, 2012/05/01 03:27:07]

I've moved the logic for using the extension url lower. It should be a fallback
for when there is no other indication, as it's intent is more like a 'browser
mode fullscreen'.

That said, the 2 states you mentioned would evaluate the same either way, the
mouse lock api would win. I've tested this manually to see that the host
evaluates to a reasonable string when called from extensions.

+    url = mouse_lock_tab_->web_contents()->GetURL();
 
-  window_->UpdateFullscreenExitBubbleContent(url,
-                                             GetFullscreenExitBubbleType());
+  FullscreenExitBubbleType bubble_type = GetFullscreenExitBubbleType();
+
+  // If bubble displays buttons, unlock mouse to allow pressing them.
+  if (fullscreen_bubble::ShowButtonsForType(bubble_type)
+      && IsMouseLocked()

[yzshen1, 2012/04/30 19:32:40]

&& at the end of the previous line, please.

[scheib, 2012/05/01 03:27:07]

Done.

+      && mouse_lock_tab_->web_contents()) {
+    mouse_lock_tab_->web_contents()->GotResponseToLockMouseRequest(false);

[yzshen1, 2012/04/30 19:32:40]

GotResponseToLockMouseRequest() should only be called when REQUESTED. At least
that is the original design, maybe you want to do what line 318 does, or change
the meaning/name of GotResponseToLockMouseRequest. (It is good to unify the
behavior with line 318 as well.)

Besides, you should change mouse_lock_state_ and mouse_lock_tab_ here.

[scheib, 2012/05/01 03:27:07]

Done. Changed to calling RVHV::UnlockMouse().

+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&FullscreenController::NotifyMouseLockChange, this));
+  }
+
+  window_->UpdateFullscreenExitBubbleContent(url, bubble_type);
 }
 
 void FullscreenController::NotifyFullscreenChange() {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_FULLSCREEN_CHANGED,
       content::Source<FullscreenController>(this),
       content::NotificationService::NoDetails());
 }
 
+void FullscreenController::NotifyMouseLockChange() {
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_MOUSE_LOCK_CHANGED,
+      content::Source<FullscreenController>(this),
+      content::NotificationService::NoDetails());
+}
+
 FullscreenExitBubbleType FullscreenController::GetFullscreenExitBubbleType()
     const {
-  if (!fullscreened_tab_) {
-    DCHECK_EQ(MOUSELOCK_NOT_REQUESTED, mouse_lock_state_);
-    return !extension_caused_fullscreen_.is_empty() ?
-        FEB_TYPE_BROWSER_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION :
-        FEB_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION;
+  if (fullscreened_tab_) {
+    if (tab_fullscreen_accepted_) {
+      if (IsMouseLocked()) {
+        return FEB_TYPE_FULLSCREEN_MOUSELOCK_EXIT_INSTRUCTION;
+      } else if (IsMouseLockRequested()) {
+        return FEB_TYPE_MOUSELOCK_BUTTONS;
+      } else {
+        return FEB_TYPE_FULLSCREEN_EXIT_INSTRUCTION;
+      }
+    } else {  // Full screen not yet accepted.
+      if (IsMouseLockRequested())
+        return FEB_TYPE_FULLSCREEN_MOUSELOCK_BUTTONS;
+      else
+        return FEB_TYPE_FULLSCREEN_BUTTONS;
+    }
+  } else if (!extension_caused_fullscreen_.is_empty()) {
+    return FEB_TYPE_BROWSER_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION;

[yzshen1, 2012/04/30 19:32:40]

Here: what if the mouse lock is requested, etc.?

[scheib, 2012/05/01 03:27:07]

Done. This should be used similar to Browser mode fullscreen, I have moved it
lower.

+  } else {  // Not a full screen tab.
+    if (IsMouseLocked()) {
+      return FEB_TYPE_MOUSELOCK_EXIT_INSTRUCTION;
+    } else if (IsMouseLockRequested()) {
+      return FEB_TYPE_MOUSELOCK_BUTTONS;
+    } else {
+      if (in_or_entering_fullscreen_) {
+        return FEB_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION;
+      } else {
+        return FEB_TYPE_NONE;
+      }
+    }
   }
-  if (fullscreened_tab_ && !tab_fullscreen_accepted_) {
-    DCHECK_NE(MOUSELOCK_ACCEPTED, mouse_lock_state_);
-    return mouse_lock_state_ == MOUSELOCK_REQUESTED ?
-        FEB_TYPE_FULLSCREEN_MOUSELOCK_BUTTONS : FEB_TYPE_FULLSCREEN_BUTTONS;
-  }
-  if (mouse_lock_state_ == MOUSELOCK_REQUESTED)
-    return FEB_TYPE_MOUSELOCK_BUTTONS;
-  return mouse_lock_state_ == MOUSELOCK_ACCEPTED ?
-      FEB_TYPE_FULLSCREEN_MOUSELOCK_EXIT_INSTRUCTION :
-      FEB_TYPE_FULLSCREEN_EXIT_INSTRUCTION;
+  NOTREACHED();
+  return FEB_TYPE_NONE;
 }
 
 ContentSetting
     FullscreenController::GetFullscreenSetting(const GURL& url) const {
   if (url.SchemeIsFile())
     return CONTENT_SETTING_ALLOW;
 
   return profile_->GetHostContentSettingsMap()->GetContentSetting(url, url,
       CONTENT_SETTINGS_TYPE_FULLSCREEN, std::string());
 
 }
 
 ContentSetting
     FullscreenController::GetMouseLockSetting(const GURL& url) const {
   if (url.SchemeIsFile())
     return CONTENT_SETTING_ALLOW;
 
   HostContentSettingsMap* settings_map = profile_->GetHostContentSettingsMap();
   return settings_map->GetContentSetting(url, url,
       CONTENT_SETTINGS_TYPE_MOUSELOCK, std::string());
 }
 
 #if defined(OS_MACOSX)
 void FullscreenController::TogglePresentationModeInternal(bool for_tab) {
-  bool entering_fullscreen = !window_->InPresentationMode();
+  in_or_entering_fullscreen_ = !window_->InPresentationMode();
   GURL url;
   if (for_tab) {
     url = browser_->GetSelectedWebContents()->GetURL();
-    tab_fullscreen_accepted_ = entering_fullscreen &&
+    tab_fullscreen_accepted_ = in_or_entering_fullscreen_ &&
         GetFullscreenSetting(url) == CONTENT_SETTING_ALLOW;
   }
-  if (entering_fullscreen)
+  if (in_or_entering_fullscreen_)
     window_->EnterPresentationMode(url, GetFullscreenExitBubbleType());
   else
     window_->ExitPresentationMode();
   WindowFullscreenStateChanged();
 }
 #endif
 
 // TODO(koz): Change |for_tab| to an enum.
 void FullscreenController::ToggleFullscreenModeInternal(bool for_tab) {
-  bool entering_fullscreen = !window_->IsFullscreen();
+  in_or_entering_fullscreen_ = !window_->IsFullscreen();

[yzshen1, 2012/04/30 19:32:40]

Is it possible that there are paths to change fullscreen state that don't go
through TogglePresentationModeInternal/ToggleFullscreenModeInternal?
(I think there are. Say, tab is closed.)

In that case, in_or_entering_fullscreen_ doesn't reflect the reality any more.

[scheib, 2012/05/01 03:27:07]

Closing a tab does call Toggle, and this state is correct / consistent within
FullscreenController.

I've renamed the variable toggled_into_fullscreen_ to be explicit about what it
represents. It's currently used in only one place, selecting the bubble type
between none, browser fullscreen, extension fullscreen. I believe any outside
cause of changing the window state would have other more serious problems if not
informing the fullscreen controller. 

 
 #if !defined(OS_MACOSX)
   // In kiosk mode, we always want to be fullscreen. When the browser first
   // starts we're not yet fullscreen, so let the initial toggle go through.
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode) &&
       window_->IsFullscreen())
     return;
 #endif
 
   GURL url;
   if (for_tab) {
     url = browser_->GetSelectedWebContents()->GetURL();
-    tab_fullscreen_accepted_ = entering_fullscreen &&
+    tab_fullscreen_accepted_ = in_or_entering_fullscreen_ &&
         GetFullscreenSetting(url) == CONTENT_SETTING_ALLOW;
   } else {
     if (!extension_caused_fullscreen_.is_empty())
       url = extension_caused_fullscreen_;
     content::RecordAction(UserMetricsAction("ToggleFullscreen"));
   }
-  if (entering_fullscreen) {
+  if (in_or_entering_fullscreen_) {
     window_->EnterFullscreen(url, GetFullscreenExitBubbleType());
   } else {
     window_->ExitFullscreen();
     extension_caused_fullscreen_ = GURL();
   }
+  UpdateFullscreenExitBubbleContent();
 
   // Once the window has become fullscreen it'll call back to
   // WindowFullscreenStateChanged(). We don't do this immediately as
   // BrowserWindow::EnterFullscreen() asks for bookmark_bar_state_, so we let
   // the BrowserWindow invoke WindowFullscreenStateChanged when appropriate.
 
   // TODO: convert mac to invoke WindowFullscreenStateChanged once it updates
   // the necessary state of the frame.
 #if defined(OS_MACOSX)
   WindowFullscreenStateChanged();
 #endif
 }