Add command line flag for disabling some Ash-specific logic for

arranging new Browser windows.

This also reinstates some logic for Mash that doesn't depend on code
in ash/. With this flag passed, or when running in Mash,
ash::WindowPositioner won't be used for placing new Windows. Some
other Chrome OS specific code, like maximizing the first run window
according to policy, is retained.

Bug: 764009
Change-Id: I7563b0b0648e2470ec4a601805d2b351b6d64bb0
Reviewed-on: https://chromium-review.googlesource.com/958030
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542858}

chrome/browser/about_flags.cc

@@ -1558,6 +1558,10 @@ const FeatureEntry kFeatureEntries[] = {
     {"show-touch-hud", flag_descriptions::kShowTouchHudName,
      flag_descriptions::kShowTouchHudDescription, kOsCrOS,
      SINGLE_VALUE_TYPE(ash::switches::kAshTouchHud)},
+    {"skip-extra-ash-window-positioning",
+     flag_descriptions::kSkipExtraAshWindowPositioningName,
+     flag_descriptions::kSkipExtraAshWindowPositioningDescription, kOsCrOS,
+     SINGLE_VALUE_TYPE(switches::kSkipExtraAshWindowPositioning)},
 #endif  // OS_CHROMEOS
     {
         "disable-accelerated-video-decode",

chrome/browser/flag_descriptions.cc

@@ -2781,6 +2781,11 @@ const char kSysInternalsDescription[] =
     "If enabled, user can monitor system information at "
     "chrome://sys-internals.";
 
+const char kSkipExtraAshWindowPositioningName[] =
+    "Simplified browser window positioning";
+const char kSkipExtraAshWindowPositioningDescription[] =
+    "Skips over Ash-specific positioning logic for new browser windows.";
+
 const char kTeamDrivesName[] = "Enable Team Drives Integration";
 const char kTeamDrivesDescription[] =
     "If enabled, files under Team Drives will appear in the Files app.";

chrome/browser/flag_descriptions.h

@@ -1707,6 +1707,9 @@ extern const char kQuickUnlockFingerprintDescription[];
 extern const char kOfficeEditingComponentAppName[];
 extern const char kOfficeEditingComponentAppDescription[];
 
+extern const char kSkipExtraAshWindowPositioningName[];
+extern const char kSkipExtraAshWindowPositioningDescription[];
+
 extern const char kTeamDrivesName[];
 extern const char kTeamDrivesDescription[];
 

chrome/browser/ui/window_sizer/window_sizer_ash.cc

@@ -7,18 +7,56 @@
 #include "ash/shell.h"
 #include "ash/wm/window_positioner.h"
 #include "ash/wm/window_state.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
+#include "components/user_manager/user.h"
+#include "components/user_manager/user_manager.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 
+namespace {
+
+// When the screen is this width or narrower, the initial browser launched on
+// first run will be maximized. TODO(estade): de-dupe with ash::WindowPositioner
+// constant of same name.
+constexpr int kForceMaximizeWidthLimit = 1366;
+
+// Returns whether WindowSizer should rely on ash::WindowPositioner. When false,
+// window sizing logic will be more similar to that of desktop platforms.
+bool ShouldConsultAsh() {
+  // TODO(crbug.com/764009): If kSkipExtraAshWindowPositioning can be made
+  // default, this will no longer need to worry about Mash.
+  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kSkipExtraAshWindowPositioning) &&
+         !ash_util::IsRunningInMash();
+}
+
+// TODO(estade): this is copied from ChromeShellDelegate. De-dupe this when
+// removing kSkipExtraAshWindowPositioning.
+bool ShouldForceMaximizeOnFirstRun() {
+  const user_manager::User* const user =
+      user_manager::UserManager::Get()->GetActiveUser();
+  if (user) {
+    return chromeos::ProfileHelper::Get()
+        ->GetProfileByUser(user)
+        ->GetPrefs()
+        ->GetBoolean(prefs::kForceMaximizeOnFirstRun);
+  }
+  return false;
+}
+
+}  // namespace
+
 bool WindowSizer::GetBrowserBoundsAsh(gfx::Rect* bounds,
                                       ui::WindowShowState* show_state) const {
-  // TODO(crbug.com/764009): Mash support.
-  if (ash_util::IsRunningInMash() || !browser_)
+  if (!browser_)
     return false;
 
   // This should not be called on a Browser that already has a window.
@@ -42,7 +80,8 @@ bool WindowSizer::GetBrowserBoundsAsh(gfx::Rect* bounds,
       state_provider_->GetPersistentState(&ignored_bounds, &ignored_work_area,
                                           show_state);
     }
-  } else if (browser_->is_type_popup() && bounds->origin().IsOrigin()) {
+  } else if (ShouldConsultAsh() && browser_->is_type_popup() &&
+             bounds->origin().IsOrigin()) {
     // 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.
@@ -105,8 +144,19 @@ void WindowSizer::GetTabbedBrowserBoundsAsh(
     return;
   }
 
-  ash::WindowPositioner::GetBoundsAndShowStateForNewWindow(
-      is_saved_bounds, passed_show_state, bounds_in_screen, show_state);
+  if (ShouldConsultAsh()) {
+    ash::WindowPositioner::GetBoundsAndShowStateForNewWindow(
+        is_saved_bounds, passed_show_state, bounds_in_screen, show_state);
+  } else if (BrowserList::GetInstance()->empty() && !is_saved_bounds &&
+             (ShouldForceMaximizeOnFirstRun() ||
+              display.work_area().width() <= kForceMaximizeWidthLimit)) {
+    // No browsers, no saved bounds: assume first run. Maximize if set by policy
+    // or if the screen is narrower than a predetermined size.
+    // TODO(estade): this logic is copied out of
+    // GetBoundsAndShowStateForNewWindow() and should be de-duped when
+    // kSkipExtraAshWindowPositioning is removed.
+    *show_state = ui::SHOW_STATE_MAXIMIZED;
+  }
 }
 
 gfx::Rect WindowSizer::GetDefaultWindowBoundsAsh(

chrome/common/chrome_switches.cc

@@ -804,6 +804,10 @@ const char kDisableLoginScreenApps[] = "disable-login-screen-apps";
 // NOTE: Used by the Chrome OS crash_reporter to identify mash processes. If you
 // change or remove the flag please update platform2/crash_reporter.
 const char kMashServiceName[] = "mash-service-name";
+
+// Disables extra Ash-specific rules for positioning new browser windows.
+const char kSkipExtraAshWindowPositioning[] =
+    "skip-extra-ash-window-positioning";
 #endif  // defined(OS_CHROMEOS)
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_CHROMEOS)

chrome/common/chrome_switches.h

@@ -249,6 +249,7 @@ extern const char kCroshCommand[];
 extern const char kDisableLoggingRedirect[];
 extern const char kDisableLoginScreenApps[];
 extern const char kMashServiceName[];
+extern const char kSkipExtraAshWindowPositioning[];
 #endif  // defined(OS_CHROMEOS)
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_CHROMEOS)

tools/metrics/histograms/enums.xml

@@ -25815,6 +25815,7 @@ from previous Chrome versions.
   <int value="-1703709912" label="enable-new-ntp"/>
   <int value="-1703308540" label="disable-webaudio"/>
   <int value="-1696366449" label="disable-permissions-bubbles"/>
+  <int value="-1695774453" label="skip-extra-ash-window-positioning"/>
   <int value="-1694353093" label="ArcVpn:disabled"/>
   <int value="-1692384483" label="disambiguate-autofill-server-name-types"/>
   <int value="-1691892152" label="SafeSearchUrlReporting:disabled"/>