More cleanup in window_sizer/window_sizer_ash

* The bounds obtained from GetBoundsOverrideAsh is ignored
  (overridden) by the following logic if it returns false,
  so skip early for the condition that returns false.
* Removed the code for non tabbed case in GetBoundsOverrideAsh as it won't be used.

* removed fullscreen controller check because it seems to be
  redundant with window->IsFullscreen() check. (IsFullscerenForBrowser() is always false if window->IsFullscreen() is false)

BUG=272460
R=skuhne@chromium.org, sky@chromium.org

Review URL: https://codereview.chromium.org/23567007

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

chrome/browser/ui/window_sizer/window_sizer.cc

@@ -98,10 +98,11 @@ class DefaultStateProvider : public WindowSizer::StateProvider {
     if (browser_ && browser_->window()) {
       window = browser_->window();
     } else {
-      // This code is only ran on the native desktop (on the ash desktop,
-      // GetBoundsOverrideAsh should take over below before this is reached).
-      // TODO(gab): This code should go in a native desktop specific window
-      // sizer as part of fixing crbug.com/175812.
+      // This code is only ran on the native desktop (on the ash
+      // desktop, GetTabbedBrowserBoundsAsh should take over below
+      // before this is reached).  TODO(gab): This code should go in a
+      // native desktop specific window sizer as part of fixing
+      // crbug.com/175812.
       const BrowserList* native_browser_list =
           BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_NATIVE);
       for (BrowserList::const_reverse_iterator it =
@@ -193,11 +194,17 @@ void WindowSizer::DetermineWindowBoundsAndShowState(
   if (bounds->IsEmpty()) {
 #if defined(USE_ASH)
     // See if ash should decide the window placement.
-    // TODO(beng): insufficient but currently necessary.
-    // http://crbug.com/133312
-    if (chrome::ShouldOpenAshOnStartup() &&
-        GetBoundsOverrideAsh(bounds, show_state))
+    if (IsTabbedBrowserInAsh()) {
+      GetTabbedBrowserBoundsAsh(bounds, show_state);
       return;
+    } else if (chrome::ShouldOpenAshOnStartup() &&
+               browser_ && browser_->host_desktop_type() ==
+               chrome::HOST_DESKTOP_TYPE_ASH) {
+      // Saved bounds's show state takes precedence over one in last
+      // bounds in ash.  If you have a question or an issue, please
+      // contact oshima@chromium.org.
+      GetSavedWindowBounds(bounds, show_state);
+    }
 #endif
     // See if there's last active window's placement information.
     if (GetLastWindowBounds(bounds, show_state))
@@ -213,9 +220,7 @@ void WindowSizer::DetermineWindowBoundsAndShowState(
     // In case of a popup with an 'unspecified' location in ash, we are
     // looking for a good screen location. We are interpreting (0,0) as an
     // unspecified location.
-    if (chrome::ShouldOpenAshOnStartup() &&
-        browser_ && browser_->is_type_popup() &&
-        bounds->x() == 0 && bounds->y() == 0) {
+    if (IsPopupBrowserInAsh() && bounds->origin().IsOrigin()) {
       *bounds = ChromeShellDelegate::instance()->window_positioner()->
           GetPopupPosition(*bounds);
       return;
@@ -399,3 +404,21 @@ ui::WindowShowState WindowSizer::GetWindowDefaultShowState() const {
   // Otherwise we use the default which can be overridden later on.
   return ui::SHOW_STATE_DEFAULT;
 }
+
+#if defined(USE_ASH)
+bool WindowSizer::IsTabbedBrowserInAsh() const {
+  // TODO(beng): insufficient but currently necessary. http://crbug.com/133312
+  return chrome::ShouldOpenAshOnStartup() &&
+      browser_ &&
+      browser_->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH &&
+      browser_->is_type_tabbed();
+}
+
+bool WindowSizer::IsPopupBrowserInAsh() const {
+  // TODO(beng): insufficient but currently necessary. http://crbug.com/133312
+  return chrome::ShouldOpenAshOnStartup() &&
+      browser_ &&
+      browser_->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH &&
+      browser_->is_type_popup();
+}
+#endif

chrome/browser/ui/window_sizer/window_sizer.h

@@ -155,20 +155,23 @@ class WindowSizer {
       gfx::Rect* bounds) const;
 
 #if defined(USE_ASH)
-  // Determines the position and size for an ash window as it gets created. This
-  // will be called before other standard placement logic. It will return true
-  // when the function was setting the bounds structure to the desired size.
-  // Otherwise another algorithm should get used to determine the correct
-  // bounds. |show_state| will only be changed if it was set to
-  // SHOW_STATE_DEFAULT.
-  bool GetBoundsOverrideAsh(gfx::Rect* bounds_in_screen,
-                            ui::WindowShowState* show_state) const;
+  // Determines the position and size for a tabbed browser window in
+  // ash as it gets created. This will be called before other standard
+  // placement logic. |show_state| will only be changed
+  // if it was set to SHOW_STATE_DEFAULT.
+  void GetTabbedBrowserBoundsAsh(gfx::Rect* bounds_in_screen,
+                                 ui::WindowShowState* show_state) const;
 #endif
 
   // Determine the default show state for the window - not looking at other
   // windows or at persistent information.
   ui::WindowShowState GetWindowDefaultShowState() const;
 
+#if defined(USE_ASH)
+  bool IsTabbedBrowserInAsh() const;
+  bool IsPopupBrowserInAsh() const;
+#endif
+
   // Providers for persistent storage and monitor metrics.
   scoped_ptr<StateProvider> state_provider_;
   gfx::Screen* screen_;  // not owned.

chrome/browser/ui/window_sizer/window_sizer_ash.cc

@@ -11,12 +11,9 @@
 #include "ash/wm/workspace/auto_window_management.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/fullscreen/fullscreen_controller.h"
-#include "chrome/browser/ui/host_desktop.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_delegate.h"
@@ -89,85 +86,66 @@ int WindowSizer::GetForceMaximizedWidthLimit() {
   return maximum_limit;
 }
 
-bool WindowSizer::GetBoundsOverrideAsh(gfx::Rect* bounds_in_screen,
-                                       ui::WindowShowState* show_state) const {
+void WindowSizer::GetTabbedBrowserBoundsAsh(
+    gfx::Rect* bounds_in_screen,
+    ui::WindowShowState* show_state) const {
   DCHECK(show_state);
   DCHECK(bounds_in_screen);
+  DCHECK(!browser_ || browser_->is_type_tabbed());
 
-  if (browser_ &&
-      browser_->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH) {
-    return false;
-  }
   bounds_in_screen->SetRect(0, 0, 0, 0);
 
-  // Experiment: Force the maximize mode for all windows.
-  if (ash::Shell::IsForcedMaximizeMode()) {
-    // Exceptions: Do not maximize popups and do not maximize windowed V1 apps
-    // which explicitly specify a |show_state| (they might be tuned for a
-    // particular resolution / type).
-    bool is_tabbed = browser_ && browser_->is_type_tabbed();
-    bool is_popup = browser_ && browser_->is_type_popup();
-    if (!is_popup && (is_tabbed || *show_state == ui::SHOW_STATE_DEFAULT))
-      *show_state = ui::SHOW_STATE_MAXIMIZED;
-  }
+  // Experiment: Force the maximize mode for all tabbed windows
+  if (ash::Shell::IsForcedMaximizeMode())
+    *show_state = ui::SHOW_STATE_MAXIMIZED;
 
   ui::WindowShowState passed_show_state = *show_state;
   bool has_saved_bounds = true;
   if (!GetSavedWindowBounds(bounds_in_screen, show_state)) {
     has_saved_bounds = false;
-    GetDefaultWindowBounds(bounds_in_screen);
+    GetDefaultWindowBoundsAsh(bounds_in_screen);
   }
 
-  if (browser_ && browser_->is_type_tabbed()) {
-    aura::RootWindow* active = ash::Shell::GetActiveRootWindow();
-    // Always open new window in the active display.
-    gfx::Rect work_area =
-        screen_->GetDisplayMatching(active->GetBoundsInScreen()).work_area();
-
-    // This is a window / app. See if there is no window and try to place it.
-    int count = GetNumberOfValidTopLevelBrowserWindows(work_area);
-    aura::Window* top_window = ash::GetTopWindowForNewWindow(active);
-    // Our window should not have any impact if we are already on top.
-    if (browser_->window() &&
-        top_window == browser_->window()->GetNativeWindow())
-      top_window = NULL;
-
-    // If there is no valid other window we take the coordinates as is.
-    if ((!count || !top_window)) {
-      if (has_saved_bounds) {
-        // Restore to previous state - if there is one.
-        bounds_in_screen->AdjustToFit(work_area);
-        return true;
-      }
-      // When using "small screens" we want to always open in full screen mode.
-      if (passed_show_state == ui::SHOW_STATE_DEFAULT &&
-          !browser_->is_session_restore() &&
-          work_area.width() <= GetForceMaximizedWidthLimit() &&
-          (!browser_->window() || !browser_->window()->IsFullscreen()) &&
-          (!browser_->fullscreen_controller() ||
-           !browser_->fullscreen_controller()->IsFullscreenForBrowser()))
-        *show_state = ui::SHOW_STATE_MAXIMIZED;
-      return true;
-    }
-    bool maximized = ash::wm::IsWindowMaximized(top_window);
-    // We ignore the saved show state, but look instead for the top level
-    // window's show state.
-    if (passed_show_state == ui::SHOW_STATE_DEFAULT) {
-      *show_state = maximized ? ui::SHOW_STATE_MAXIMIZED :
-                                ui::SHOW_STATE_DEFAULT;
+  aura::RootWindow* active = ash::Shell::GetActiveRootWindow();
+  // Always open new window in the active display.
+  gfx::Rect work_area =
+      screen_->GetDisplayMatching(active->GetBoundsInScreen()).work_area();
+
+  // This is a window / app. See if there is no window and try to place it.
+  int count = GetNumberOfValidTopLevelBrowserWindows(work_area);
+  aura::Window* top_window = ash::GetTopWindowForNewWindow(active);
+  // Our window should not have any impact if we are already on top.
+  if (browser_->window() &&
+      top_window == browser_->window()->GetNativeWindow())
+    top_window = NULL;
+
+  // If there is no valid other window we take the coordinates as is.
+  if ((!count || !top_window)) {
+    if (has_saved_bounds) {
+      // Restore to previous state - if there is one.
+      bounds_in_screen->AdjustToFit(work_area);
+      return;
     }
 
-    if (maximized)
-      return true;
-
-    // Use the size of the other window. The window's bound will be rearranged
-    // in ash::WorkspaceLayoutManager using this location.
-    *bounds_in_screen = top_window->GetBoundsInScreen();
-
-    return true;
+    // When using "small screens" we want to always open in full screen mode.
+    if (passed_show_state == ui::SHOW_STATE_DEFAULT &&
+        !browser_->is_session_restore() &&
+        work_area.width() <= GetForceMaximizedWidthLimit() &&
+        (!browser_->window() || !browser_->window()->IsFullscreen()))
+      *show_state = ui::SHOW_STATE_MAXIMIZED;
+    return;
+  }
+  bool maximized = ash::wm::IsWindowMaximized(top_window);
+  // We ignore the saved show state, but look instead for the top level
+  // window's show state.
+  if (passed_show_state == ui::SHOW_STATE_DEFAULT) {
+    *show_state = maximized ? ui::SHOW_STATE_MAXIMIZED :
+        ui::SHOW_STATE_DEFAULT;
   }
 
-  return false;
+  // Use the size of the other window. The window's bound will be rearranged
+  // in ash::WorkspaceLayoutManager using this location.
+  *bounds_in_screen = top_window->GetBoundsInScreen();
 }
 
 void WindowSizer::GetDefaultWindowBoundsAsh(gfx::Rect* default_bounds) const {