Revert 179615 - didn't help by itself. (May need another revert for r179399)

> Revert 179554 - suspected to break browser_tests on Mac 10.6
> 
> > Refactor FullscreenController removing TogglePresentationMode & adding ToggleFullscreenWithChrome.
> > 
> > Part of a series of refactoring changes that will enable simpler code in FullscreenController as well as correcting behavior there. This change attempts to make the minimal modifications required in order to have FullscreenController::ToggleFullscreenMode consistently mean fullscreen with no browser chrome on all platforms.
> >  
> > Depends on https://codereview.chromium.org/12018007/
> > 
> > BUG=169138
> > 
> > Review URL: https://chromiumcodereview.appspot.com/11896104
> 
> TBR=scheib@chromium.org
> Review URL: https://codereview.chromium.org/12089068

TBR=kinuko@chromium.org
Review URL: https://codereview.chromium.org/12096059

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

11 files changed, 108 insertions, 89 deletions

diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index cbd53a9..23fdd51 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -767,12 +767,6 @@ void Browser::ToggleFullscreenModeWithExtension(const GURL& extension_url) {
   fullscreen_controller_->ToggleFullscreenModeWithExtension(extension_url);
 }
 
-#if defined(OS_MACOSX)
-void Browser::TogglePresentationMode() {
-  fullscreen_controller_->TogglePresentationMode();
-}
-#endif
-
 bool Browser::SupportsWindowFeature(WindowFeature feature) const {
   return SupportsWindowFeatureImpl(feature, true);
 }
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index ec797fc..9f86dd5 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -346,9 +346,6 @@ class Browser : public TabStripModelObserver,
   // See the description of FullscreenController::ToggleMetroSnapMode.
   void SetMetroSnapMode(bool enable);
 #endif
-#if defined(OS_MACOSX)
-  void TogglePresentationMode();
-#endif
 
   // Returns true if the Browser supports the specified feature. The value of
   // this varies during the lifetime of the browser. For example, if the window
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
index 72ce592..60f0fe9 100644
--- a/chrome/browser/ui/browser_command_controller.cc
+++ b/chrome/browser/ui/browser_command_controller.cc
@@ -39,6 +39,10 @@
 #include "content/public/common/url_constants.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 
+#if defined(OS_MACOSX)
+#include "chrome/browser/ui/browser_commands_mac.h"
+#endif
+
 #if defined(OS_WIN)
 #include "base/win/metro.h"
 #endif
@@ -421,7 +425,11 @@ void BrowserCommandController::ExecuteCommandWithDisposition(
       ConvertPopupToTabbedBrowser(browser_);
       break;
     case IDC_FULLSCREEN:
+#if defined(OS_MACOSX)
+      chrome::ToggleFullscreenWithChrome(browser_);
+#else
       chrome::ToggleFullscreenMode(browser_);
+#endif
       break;
 
 #if defined(USE_ASH)
@@ -450,7 +458,7 @@ void BrowserCommandController::ExecuteCommandWithDisposition(
 
 #if defined(OS_MACOSX)
     case IDC_PRESENTATION_MODE:
-      browser_->TogglePresentationMode();
+      chrome::ToggleFullscreenMode(browser_);
       break;
 #endif
     case IDC_EXIT:
diff --git a/chrome/browser/ui/browser_commands_mac.cc b/chrome/browser/ui/browser_commands_mac.cc
new file mode 100644
index 0000000..bde0b5a
--- /dev/null
+++ b/chrome/browser/ui/browser_commands_mac.cc
@@ -0,0 +1,16 @@
+// Copyright (c) 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/browser_commands_mac.h"
+
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/fullscreen/fullscreen_controller.h"
+
+namespace chrome {
+
+void ToggleFullscreenWithChrome(Browser* browser) {
+  browser->fullscreen_controller()->ToggleFullscreenWithChrome();
+}
+
+}  // namespace chrome
diff --git a/chrome/browser/ui/browser_commands_mac.h b/chrome/browser/ui/browser_commands_mac.h
new file mode 100644
index 0000000..730be67
--- /dev/null
+++ b/chrome/browser/ui/browser_commands_mac.h
@@ -0,0 +1,16 @@
+// Copyright (c) 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.
+
+#ifndef CHROME_BROWSER_UI_BROWSER_COMMANDS_MAC_H_
+#define CHROME_BROWSER_UI_BROWSER_COMMANDS_MAC_H_
+
+class Browser;
+
+namespace chrome {
+
+void ToggleFullscreenWithChrome(Browser* browser);
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_UI_BROWSER_COMMANDS_MAC_H_
diff --git a/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm b/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
index 5800a80..5ca760f 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
@@ -13,6 +13,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/cocoa/browser_window_cocoa.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller_private.h"
@@ -271,7 +272,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, ZOrderNormal) {
 // Verify that in non-instant presentation mode that the info bar is below the
 // content are and everything else is above it.
 IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, ZOrderPresentationMode) {
-  browser()->TogglePresentationMode();
+  chrome::ToggleFullscreenMode(browser());
   browser()->GetFindBarController();  // add find bar
 
   std::vector<ViewID> view_list;
@@ -305,7 +306,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, ZOrderNormalInstant) {
 // non-instant presentation mode.
 IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest,
                        ZOrderInstantPresentationMode) {
-  browser()->TogglePresentationMode();
+  chrome::ToggleFullscreenMode(browser());
   ShowInstantResults();
   browser()->GetFindBarController();  // add find bar
 
@@ -353,7 +354,7 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, ContentOffset) {
 // the info bar.
 IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest,
                        ContentOffsetPresentationMode) {
-  browser()->TogglePresentationMode();
+  chrome::ToggleFullscreenMode(browser());
   PreviewableContentsController* preview =
       [controller() previewableContentsController];
 
diff --git a/chrome/browser/ui/fullscreen/fullscreen_controller.cc b/chrome/browser/ui/fullscreen/fullscreen_controller.cc
index 519adcc..6a6cf44 100644
--- a/chrome/browser/ui/fullscreen/fullscreen_controller.cc
+++ b/chrome/browser/ui/fullscreen/fullscreen_controller.cc
@@ -57,7 +57,7 @@ bool FullscreenController::IsFullscreenForBrowser() const {
 
 void FullscreenController::ToggleFullscreenMode() {
   extension_caused_fullscreen_ = GURL();
-  ToggleFullscreenModeInternal(false);
+  ToggleFullscreenModeInternal(BROWSER);
 }
 
 bool FullscreenController::IsFullscreenForTabOrPending() const {
@@ -92,11 +92,7 @@ void FullscreenController::ToggleFullscreenModeForTab(WebContents* web_contents,
     SetFullscreenedTab(web_contents);
     if (!in_browser_or_tab_fullscreen_mode) {
       tab_caused_fullscreen_ = true;
-#if defined(OS_MACOSX)
-      TogglePresentationModeInternal(true);
-#else
-      ToggleFullscreenModeInternal(true);
-#endif
+      ToggleFullscreenModeInternal(TAB);
     } else {
       // We need to update the fullscreen exit bubble, e.g., going from browser
       // fullscreen to tab fullscreen will need to show different content.
@@ -114,11 +110,7 @@ void FullscreenController::ToggleFullscreenModeForTab(WebContents* web_contents,
   } else {
     if (in_browser_or_tab_fullscreen_mode) {
       if (tab_caused_fullscreen_) {
-#if defined(OS_MACOSX)
-        TogglePresentationModeInternal(true);
-#else
-        ToggleFullscreenModeInternal(true);
-#endif
+        ToggleFullscreenModeInternal(TAB);
       } 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"
@@ -140,7 +132,7 @@ void FullscreenController::ToggleFullscreenModeWithExtension(
   // |extension_caused_fullscreen_| will be reset if this causes fullscreen to
   // exit.
   extension_caused_fullscreen_ = extension_url;
-  ToggleFullscreenModeInternal(false);
+  ToggleFullscreenModeInternal(BROWSER);
 }
 
 bool FullscreenController::IsInMetroSnapMode() {
@@ -166,8 +158,8 @@ void FullscreenController::SetMetroSnapMode(bool enable) {
 #endif  // defined(OS_WIN)
 
 #if defined(OS_MACOSX)
-void FullscreenController::TogglePresentationMode() {
-  TogglePresentationModeInternal(false);
+void FullscreenController::ToggleFullscreenWithChrome() {
+  ToggleFullscreenModeInternal(BROWSER_WITH_CHROME);
 }
 #endif
 
@@ -274,15 +266,10 @@ bool FullscreenController::HandleUserPressedEscape() {
 }
 
 void FullscreenController::ExitTabOrBrowserFullscreenToPreviousState() {
-  if (IsFullscreenForTabOrPending()) {
+  if (IsFullscreenForTabOrPending())
     ExitTabFullscreenOrMouseLockIfNecessary();
-  } else if (IsFullscreenForBrowser()) {
-#if defined(OS_MACOSX)
-    TogglePresentationMode();
-#else
-    ToggleFullscreenMode();
-#endif
-  }
+  else if (IsFullscreenForBrowser())
+    ExitFullscreenModeInternal();
 }
 
 void FullscreenController::OnAcceptFullscreenPermission() {
@@ -497,55 +484,63 @@ void FullscreenController::NotifyMouseLockChange() {
 }
 
 // TODO(koz): Change |for_tab| to an enum.
-void FullscreenController::ToggleFullscreenModeInternal(bool for_tab) {
+void FullscreenController::ToggleFullscreenModeInternal(
+    FullscreenInternalOption option) {
 #if defined(OS_WIN)
   // When in Metro snap mode, toggling in and out of fullscreen is prevented.
   if (IsInMetroSnapMode())
     return;
 #endif
 
-  toggled_into_fullscreen_ = !window_->IsFullscreen();
+  bool enter_fullscreen = !window_->IsFullscreen();
 #if defined(OS_MACOSX)
-  // When a Mac user requests a toggle they may be transitioning from
-  // FullscreenWithoutChrome to FullscreenWithChrome.
-  if (!IsFullscreenForTabOrPending())
-    toggled_into_fullscreen_ |= window_->IsFullscreenWithoutChrome();
+  // When a Mac user requests a toggle they may be toggling between
+  // FullscreenWithoutChrome and FullscreenWithChrome.
+  if (!IsFullscreenForTabOrPending()) {
+    if (option == BROWSER_WITH_CHROME)
+      enter_fullscreen |= window_->IsFullscreenWithoutChrome();
+    else
+      enter_fullscreen |= window_->IsFullscreenWithChrome();
+  }
 #endif
 
   // 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) &&
-      !toggled_into_fullscreen_)
+      window_->IsFullscreen())
     return;
 
+  if (enter_fullscreen)
+    EnterFullscreenModeInternal(option);
+  else
+    ExitFullscreenModeInternal();
+}
+
+void FullscreenController::EnterFullscreenModeInternal(
+    FullscreenInternalOption option) {
+  toggled_into_fullscreen_ = true;
   GURL url;
-  if (for_tab) {
+  if (option == TAB) {
     url = browser_->tab_strip_model()->GetActiveWebContents()->GetURL();
-    tab_fullscreen_accepted_ = toggled_into_fullscreen_ &&
+    tab_fullscreen_accepted_ =
         GetFullscreenSetting(url) == CONTENT_SETTING_ALLOW;
   } else {
     if (!extension_caused_fullscreen_.is_empty())
       url = extension_caused_fullscreen_;
     content::RecordAction(UserMetricsAction("ToggleFullscreen"));
   }
-  if (toggled_into_fullscreen_) {
+
 #if defined(OS_MACOSX)
-    CHECK(!for_tab);  // EnterFullscreenWithChrome invalid for tab fullscreen.
+  if (option == BROWSER_WITH_CHROME) {
     CHECK(base::mac::IsOSLionOrLater());
     window_->EnterFullscreenWithChrome();
-#else
-    window_->EnterFullscreen(url, GetFullscreenExitBubbleType());
-#endif
   } else {
-#if defined(OS_MACOSX)
-    // Mac windows report a state change instantly, and so we must also clear
-    // tab_caused_fullscreen_ to match them else other logic using
-    // tab_caused_fullscreen_ will be incorrect.
-    NotifyTabOfExitIfNecessary();
+#else
+  {
 #endif
-    window_->ExitFullscreen();
-    extension_caused_fullscreen_ = GURL();
+    window_->EnterFullscreen(url, GetFullscreenExitBubbleType());
   }
+
   UpdateFullscreenExitBubbleContent();
 
   // Once the window has become fullscreen it'll call back to
@@ -554,31 +549,19 @@ void FullscreenController::ToggleFullscreenModeInternal(bool for_tab) {
   // the BrowserWindow invoke WindowFullscreenStateChanged when appropriate.
 }
 
+void FullscreenController::ExitFullscreenModeInternal() {
+  toggled_into_fullscreen_ = false;
 #if defined(OS_MACOSX)
-void FullscreenController::TogglePresentationModeInternal(bool for_tab) {
-  toggled_into_fullscreen_ = !window_->IsFullscreenWithoutChrome();
-  GURL url;
-  if (for_tab) {
-    url = browser_->tab_strip_model()->GetActiveWebContents()->GetURL();
-    tab_fullscreen_accepted_ = toggled_into_fullscreen_ &&
-        GetFullscreenSetting(url) == CONTENT_SETTING_ALLOW;
-  }
-  if (!window_->IsFullscreenWithoutChrome()) {
-    window_->EnterFullscreen(url, GetFullscreenExitBubbleType());
-  } else {
-    window_->ExitFullscreen();
+  // Mac windows report a state change instantly, and so we must also clear
+  // tab_caused_fullscreen_ to match them else other logic using
+  // tab_caused_fullscreen_ will be incorrect.
+  NotifyTabOfExitIfNecessary();
+#endif
+  window_->ExitFullscreen();
+  extension_caused_fullscreen_ = GURL();
 
-    // Mac windows report a state change instantly, and so we must also clear
-    // tab_caused_fullscreen_ to match them else other logic using
-    // tab_caused_fullscreen_ will be incorrect.
-    NotifyTabOfExitIfNecessary();
-  }
   UpdateFullscreenExitBubbleContent();
-
-  // WindowFullscreenStateChanged will be called by BrowserWindowController
-  // when the transition completes.
 }
-#endif
 
 void FullscreenController::SetFullscreenedTab(WebContents* tab) {
   fullscreened_tab_ = tab;
@@ -592,7 +575,7 @@ void FullscreenController::SetMouseLockTab(WebContents* tab) {
 
 void FullscreenController::ExitTabFullscreenOrMouseLockIfNecessary() {
   if (tab_caused_fullscreen_)
-    ToggleFullscreenModeInternal(true);
+    ToggleFullscreenModeInternal(TAB);
   else
     NotifyTabOfExitIfNecessary();
 }
diff --git a/chrome/browser/ui/fullscreen/fullscreen_controller.h b/chrome/browser/ui/fullscreen/fullscreen_controller.h
index 6b094f0..8da996e 100644
--- a/chrome/browser/ui/fullscreen/fullscreen_controller.h
+++ b/chrome/browser/ui/fullscreen/fullscreen_controller.h
@@ -74,7 +74,7 @@ class FullscreenController : public content::NotificationObserver {
 #endif
 
 #if defined(OS_MACOSX)
-  void TogglePresentationMode();
+  void ToggleFullscreenWithChrome();
 #endif
 
   // Mouse Lock ////////////////////////////////////////////////////////////////
@@ -130,6 +130,14 @@ class FullscreenController : public content::NotificationObserver {
     MOUSELOCK_ACCEPTED_SILENTLY
   };
 
+  enum FullscreenInternalOption {
+    BROWSER,
+#if defined(OS_MACOSX)
+    BROWSER_WITH_CHROME,
+#endif
+    TAB
+  };
+
   void UpdateNotificationRegistrations();
 
   // Posts a task to call NotifyFullscreenChange.
@@ -141,11 +149,9 @@ class FullscreenController : public content::NotificationObserver {
   void NotifyTabOfExitIfNecessary();
   void NotifyMouseLockChange();
 
-  // TODO(koz): Change |for_tab| to an enum.
-  void ToggleFullscreenModeInternal(bool for_tab);
-#if defined(OS_MACOSX)
-  void TogglePresentationModeInternal(bool for_tab);
-#endif
+  void ToggleFullscreenModeInternal(FullscreenInternalOption option);
+  void EnterFullscreenModeInternal(FullscreenInternalOption option);
+  void ExitFullscreenModeInternal();
   void SetFullscreenedTab(content::WebContents* tab);
   void SetMouseLockTab(content::WebContents* tab);
 
diff --git a/chrome/browser/ui/fullscreen/fullscreen_controller_interactive_browsertest.cc b/chrome/browser/ui/fullscreen/fullscreen_controller_interactive_browsertest.cc
index 7fa124a..d312f82 100644
--- a/chrome/browser/ui/fullscreen/fullscreen_controller_interactive_browsertest.cc
+++ b/chrome/browser/ui/fullscreen/fullscreen_controller_interactive_browsertest.cc
@@ -379,7 +379,7 @@ IN_PROC_BROWSER_TEST_F(
 
   {
     FullscreenNotificationObserver fullscreen_observer;
-    browser()->TogglePresentationMode();
+    chrome::ToggleFullscreenMode(browser());
     fullscreen_observer.Wait();
     EXPECT_FALSE(browser()->window()->IsFullscreen());
     EXPECT_FALSE(browser()->window()->IsFullscreenWithChrome());
diff --git a/chrome/browser/ui/fullscreen/fullscreen_controller_state_test.cc b/chrome/browser/ui/fullscreen/fullscreen_controller_state_test.cc
index 473aad8..fad0d0d 100644
--- a/chrome/browser/ui/fullscreen/fullscreen_controller_state_test.cc
+++ b/chrome/browser/ui/fullscreen/fullscreen_controller_state_test.cc
@@ -277,11 +277,7 @@ bool FullscreenControllerStateTest::InvokeEvent(Event event) {
 
   switch (event) {
     case TOGGLE_FULLSCREEN:
-#if defined(OS_MACOSX)
-      GetFullscreenController()->TogglePresentationMode();
-#else
       GetFullscreenController()->ToggleFullscreenMode();
-#endif
       break;
     case TAB_FULLSCREEN_TRUE:
       GetFullscreenController()->ToggleFullscreenModeForTab(
diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi
index b455d94..ba3fcbf 100644
--- a/chrome/chrome_browser_ui.gypi
+++ b/chrome/chrome_browser_ui.gypi
@@ -245,6 +245,8 @@
         'browser/ui/browser.h',
         'browser/ui/browser_commands.cc',
         'browser/ui/browser_commands.h',
+        'browser/ui/browser_commands_mac.cc',
+        'browser/ui/browser_commands_mac.h',
         'browser/ui/browser_command_controller.cc',
         'browser/ui/browser_command_controller.h',
         'browser/ui/browser_content_setting_bubble_model_delegate.cc',