This CL does the following

- puts browser windows which would have used AppNonClientFrameViewAsh into maximized mode
- puts v1 app windows which would not have used AppNonClientFrameViewAsh and browser popup windows into tab fullscreen
- Makes Alt+ put windows into maximized mode. (That's what Ctrl+Alt+? indicates)

TEST=ChromeShellDelegateBrowserTest
BUG=251311

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

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

ash/accelerators/accelerator_controller.cc

@@ -831,10 +831,6 @@ bool AcceleratorController::PerformAction(int action,
       return true;
     }
     case TOGGLE_MAXIMIZED: {
-      if (key_code == ui::VKEY_MEDIA_LAUNCH_APP2) {
-        shell->delegate()->RecordUserMetricsAction(
-            UMA_ACCEL_MAXIMIZE_RESTORE_F4);
-      }
       shell->delegate()->ToggleMaximized();
       return true;
     }

ash/accelerators/accelerator_table.cc

@@ -109,7 +109,7 @@ const AcceleratorData kAcceleratorData[] = {
   // extended keyboard shortcuts.
   { false, ui::VKEY_LWIN, ui::EF_NONE, TOGGLE_APP_LIST },
   { false, ui::VKEY_LWIN, ui::EF_ALT_DOWN, TOGGLE_CAPS_LOCK },
-  { true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_NONE, TOGGLE_MAXIMIZED },
+  { true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_NONE, TOGGLE_FULLSCREEN },
   { true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_SHIFT_DOWN, TOGGLE_FULLSCREEN },
   { true, ui::VKEY_VOLUME_MUTE, ui::EF_NONE, VOLUME_MUTE },
   { true, ui::VKEY_VOLUME_DOWN, ui::EF_NONE, VOLUME_DOWN },

chrome/browser/ui/ash/chrome_shell_delegate.cc

@@ -159,12 +159,31 @@ void ChromeShellDelegate::ToggleFullscreen() {
   bool is_fullscreen = ash::wm::IsWindowFullscreen(window);
 
   // Windows which cannot be maximized should not be fullscreened.
-  if (is_fullscreen && !ash::wm::CanMaximizeWindow(window))
+  if (!is_fullscreen && !ash::wm::CanMaximizeWindow(window))
     return;
 
   Browser* browser = chrome::FindBrowserWithWindow(window);
   if (browser) {
-    chrome::ToggleFullscreenMode(browser);
+    // If a window is fullscreen, exit fullscreen.
+    if (is_fullscreen) {
+      chrome::ToggleFullscreenMode(browser);
+      return;
+    }
+
+    // AppNonClientFrameViewAsh shows only the window controls and no other
+    // window decorations which is pretty close to fullscreen. Put v1 apps
+    // into maximized mode instead of fullscreen to avoid showing the ugly
+    // fullscreen exit bubble.
+#if defined(OS_WIN)
+    if (browser->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_NATIVE) {
+      chrome::ToggleFullscreenMode(browser);
+      return;
+    }
+#endif  // OS_WIN
+    if (browser->is_app() && browser->app_type() != Browser::APP_TYPE_CHILD)
+      ash::wm::ToggleMaximizedWindow(window);
+    else
+      chrome::ToggleFullscreenMode(browser);
     return;
   }
 
@@ -185,13 +204,6 @@ void ChromeShellDelegate::ToggleMaximized() {
   if (!window)
     return;
 
-  // TODO(pkotwicz): If immersive mode replaces fullscreen, bind fullscreen to
-  // F4 and find a different key binding for maximize.
-  if (ImmersiveFullscreenConfiguration::UseImmersiveFullscreen()) {
-    ToggleFullscreen();
-    return;
-  }
-
   // Get out of fullscreen when in fullscreen mode.
   if (ash::wm::IsWindowFullscreen(window)) {
     ToggleFullscreen();

chrome/browser/ui/ash/chrome_shell_delegate_browsertest.cc

@@ -7,13 +7,34 @@
 #include "ash/ash_switches.h"
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
+#include "ash/wm/window_properties.h"
 #include "ash/wm/window_util.h"
 #include "base/command_line.h"
+#include "chrome/browser/extensions/platform_app_browsertest_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/extensions/native_app_window.h"
 #include "chrome/browser/ui/immersive_fullscreen_configuration.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "ui/aura/client/aura_constants.h"
+
+namespace {
+
+// Returns true if |window| is in immersive fullscreen. Infer whether |window|
+// is in immersive fullscreen based on whether kFullscreenUsesMinimalChromeKey
+// is set for |window| because DEPS does not allow the test to use BrowserView.
+// (This is not quite right because if a window is in both immersive browser
+// fullscreen and in tab fullscreen, kFullScreenUsesMinimalChromeKey will
+// not be set).
+bool IsInImmersiveFullscreen(BrowserWindow* browser_window) {
+  return browser_window->IsFullscreen() &&
+      browser_window->GetNativeWindow()->GetProperty(
+          ash::internal::kFullscreenUsesMinimalChromeKey);
+}
+
+}  // namespace
 
 typedef InProcessBrowserTest ChromeShellDelegateBrowserTest;
 
@@ -24,17 +45,6 @@ IN_PROC_BROWSER_TEST_F(ChromeShellDelegateBrowserTest, ToggleMaximized) {
   aura::Window* window = ash::wm::GetActiveWindow();
   ASSERT_TRUE(window);
 
-  if (ImmersiveFullscreenConfiguration::UseImmersiveFullscreen()) {
-    // "ToggleMaximized" toggles immersive fullscreen.
-    EXPECT_FALSE(ash::wm::IsWindowMaximized(window));
-    EXPECT_FALSE(ash::wm::IsWindowFullscreen(window));
-    shell_delegate->ToggleMaximized();
-    EXPECT_TRUE(ash::wm::IsWindowFullscreen(window));
-    shell_delegate->ToggleMaximized();
-    EXPECT_FALSE(ash::wm::IsWindowFullscreen(window));
-    return;
-  }
-
   // When not in fullscreen, ShellDelegate::ToggleMaximized toggles Maximized.
   EXPECT_FALSE(ash::wm::IsWindowMaximized(window));
   shell_delegate->ToggleMaximized();
@@ -55,3 +65,126 @@ IN_PROC_BROWSER_TEST_F(ChromeShellDelegateBrowserTest, ToggleMaximized) {
   EXPECT_FALSE(ash::wm::IsWindowFullscreen(window));
   EXPECT_TRUE(ash::wm::IsWindowMaximized(window));
 }
+
+// Confirm that toggling window fullscren works properly.
+IN_PROC_BROWSER_TEST_F(ChromeShellDelegateBrowserTest, ToggleFullscreen) {
+  ash::ShellDelegate* shell_delegate = ash::Shell::GetInstance()->delegate();
+  ASSERT_TRUE(shell_delegate);
+
+  // 1) ToggleFullscreen() should toggle whether a tabbed browser window is in
+  // immersive fullscreen.
+  ASSERT_TRUE(browser()->is_type_tabbed());
+  BrowserWindow* browser_window = browser()->window();
+  ASSERT_TRUE(browser_window->IsActive());
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_TRUE(browser_window->IsFullscreen());
+  EXPECT_EQ(IsInImmersiveFullscreen(browser_window),
+      ImmersiveFullscreenConfiguration::UseImmersiveFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  // 2) ToggleFullscreen() should have no effect on windows which cannot be
+  // maximized.
+  browser_window->GetNativeWindow()->SetProperty(aura::client::kCanMaximizeKey,
+                                                 false);
+  shell_delegate->ToggleFullscreen();
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  // 3) ToggleFullscreen() should maximize v1 app browser windows which use
+  // AppNonClientFrameViewAsh.
+  // TODO(pkotwicz): Figure out if we actually want this behavior.
+  Browser::CreateParams browser_create_params(Browser::TYPE_POPUP,
+      browser()->profile(), chrome::HOST_DESKTOP_TYPE_NATIVE);
+#if defined(OS_WIN)
+  browser_create_params.host_desktop_type = chrome::HOST_DESKTOP_TYPE_ASH;
+#endif  // OS_WIN
+  browser_create_params.app_name = "Test";
+  browser_create_params.app_type = Browser::APP_TYPE_HOST;
+
+  Browser* app_host_browser = new Browser(browser_create_params);
+  ASSERT_TRUE(app_host_browser->is_app());
+  AddBlankTabAndShow(app_host_browser);
+  browser_window = app_host_browser->window();
+  ASSERT_TRUE(browser_window->IsActive());
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_TRUE(browser_window->IsMaximized());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  // 4) ToggleFullscreen() should put child windows of v1 apps into
+  // non-immersive fullscreen.
+  browser_create_params.host_desktop_type = chrome::HOST_DESKTOP_TYPE_NATIVE;
+  browser_create_params.app_type = Browser::APP_TYPE_CHILD;
+  Browser* app_child_browser = new Browser(browser_create_params);
+  ASSERT_TRUE(app_child_browser->is_app());
+  AddBlankTabAndShow(app_child_browser);
+  browser_window = app_child_browser->window();
+  ASSERT_TRUE(browser_window->IsActive());
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_TRUE(browser_window->IsFullscreen());
+  EXPECT_FALSE(IsInImmersiveFullscreen(browser_window));
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  // 5) ToggleFullscreen() should put popup browser windows into non-immersive
+  // fullscreen.
+  browser_create_params.app_name = "";
+  Browser* popup_browser = new Browser(browser_create_params);
+  ASSERT_TRUE(popup_browser->is_type_popup());
+  ASSERT_FALSE(popup_browser->is_app());
+  AddBlankTabAndShow(popup_browser);
+  browser_window = popup_browser->window();
+  ASSERT_TRUE(browser_window->IsActive());
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_TRUE(browser_window->IsFullscreen());
+  EXPECT_FALSE(IsInImmersiveFullscreen(browser_window));
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_FALSE(browser_window->IsMaximized());
+  EXPECT_FALSE(browser_window->IsFullscreen());
+}
+
+typedef extensions::PlatformAppBrowserTest
+    ChromeShellDelegatePlatformAppBrowserTest;
+
+// Test that ToggleFullscreen() toggles the platform app's fullscreen state.
+IN_PROC_BROWSER_TEST_F(ChromeShellDelegatePlatformAppBrowserTest,
+                       ToggleFullscreenPlatformApp) {
+  ash::ShellDelegate* shell_delegate = ash::Shell::GetInstance()->delegate();
+  ASSERT_TRUE(shell_delegate);
+
+  const extensions::Extension* extension = LoadAndLaunchPlatformApp("minimal");
+  ShellWindow* shell_window = CreateShellWindow(extension);
+  NativeAppWindow* app_window = shell_window->GetBaseWindow();
+  ASSERT_TRUE(shell_window->GetBaseWindow()->IsActive());
+  EXPECT_FALSE(app_window->IsMaximized());
+  EXPECT_FALSE(app_window->IsFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_TRUE(app_window->IsFullscreen());
+
+  shell_delegate->ToggleFullscreen();
+  EXPECT_FALSE(app_window->IsMaximized());
+  EXPECT_FALSE(app_window->IsFullscreen());
+
+  CloseShellWindow(shell_window);
+}

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

@@ -248,15 +248,6 @@ bool ShouldSaveOrRestoreWindowPos() {
   return true;
 }
 
-// Returns whether immersive mode should replace fullscreen, which should only
-// occur for "browser-fullscreen" and not for "tab-fullscreen" (which has a URL
-// for the tab entering fullscreen).
-bool UseImmersiveFullscreenForUrl(const GURL& url) {
-  bool is_browser_fullscreen = url.is_empty();
-  return is_browser_fullscreen &&
-         ImmersiveFullscreenConfiguration::UseImmersiveFullscreen();
-}
-
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -886,7 +877,7 @@ void BrowserView::UpdateFullscreenExitBubbleContent(
   // Immersive mode has no exit bubble because it has a visible strip at the
   // top that gives the user a hover target.
   // TODO(jamescook): Figure out what to do with mouse-lock.
-  if (bubble_type == FEB_TYPE_NONE || UseImmersiveFullscreenForUrl(url)) {
+  if (bubble_type == FEB_TYPE_NONE || ShouldUseImmersiveFullscreenForUrl(url)) {
     fullscreen_bubble_.reset();
   } else if (fullscreen_bubble_.get()) {
     fullscreen_bubble_->UpdateContent(url, bubble_type);
@@ -2389,7 +2380,7 @@ void BrowserView::ProcessFullscreen(bool fullscreen,
   }
 
   // Enable immersive before the browser refreshes its list of enabled commands.
-  if (UseImmersiveFullscreenForUrl(url))
+  if (ShouldUseImmersiveFullscreenForUrl(url))
     immersive_mode_controller_->SetEnabled(fullscreen);
 
   browser_->WindowFullscreenStateChanged();
@@ -2397,7 +2388,7 @@ void BrowserView::ProcessFullscreen(bool fullscreen,
   if (fullscreen) {
     if (!chrome::IsRunningInAppMode() &&
         type != FOR_METRO &&
-        !UseImmersiveFullscreenForUrl(url)) {
+        !ShouldUseImmersiveFullscreenForUrl(url)) {
       fullscreen_bubble_.reset(new FullscreenExitBubbleViews(
           this, url, bubble_type));
     }
@@ -2420,6 +2411,12 @@ void BrowserView::ProcessFullscreen(bool fullscreen,
   ToolbarSizeChanged(false);
 }
 
+bool BrowserView::ShouldUseImmersiveFullscreenForUrl(const GURL& url) const {
+  bool is_browser_fullscreen = url.is_empty();
+  return ImmersiveFullscreenConfiguration::UseImmersiveFullscreen() &&
+      is_browser_fullscreen && IsBrowserTypeNormal();
+}
+
 void BrowserView::LoadAccelerators() {
 #if defined(OS_WIN) && !defined(USE_AURA)
   HACCEL accelerator_table = AtlLoadAccelerators(IDR_MAINFRAME);

chrome/browser/ui/views/frame/browser_view.h

@@ -551,6 +551,11 @@ class BrowserView : public BrowserWindow,
                          const GURL& url,
                          FullscreenExitBubbleType bubble_type);
 
+  // Returns whether immmersive fullscreen should replace fullscreen. This
+  // should only occur for "browser-fullscreen" for tabbed-typed windows (not
+  // for tab-fullscreen and not for app/popup type windows).
+  bool ShouldUseImmersiveFullscreenForUrl(const GURL& url) const;
+
   // Copy the accelerator table from the app resources into something we can
   // use.
   void LoadAccelerators();