Override MAXIMIZED window show state if window size / position is manually specified.

BUG=715729

Review-Url: https://codereview.chromium.org/2844843002
Cr-Commit-Position: refs/heads/master@{#468321}

chrome/browser/ui/browser_window_state.cc

@@ -161,20 +161,38 @@ void GetSavedWindowBoundsAndShowState(const Browser* browser,
   const base::CommandLine& parsed_command_line =
       *base::CommandLine::ForCurrentProcess();
 
-  if (parsed_command_line.HasSwitch(switches::kWindowSize)) {
-    std::string str =
-        parsed_command_line.GetSwitchValueASCII(switches::kWindowSize);
+  internal::UpdateWindowBoundsAndShowStateFromCommandLine(parsed_command_line,
+                                                          bounds, show_state);
+}
+
+namespace internal {
+
+void UpdateWindowBoundsAndShowStateFromCommandLine(
+    const base::CommandLine& command_line,
+    gfx::Rect* bounds,
+    ui::WindowShowState* show_state) {
+  // Allow command-line flags to override the window size and position. If
+  // either of these is specified then set the show state to NORMAL so that
+  // they are immediately respected.
+  if (command_line.HasSwitch(switches::kWindowSize)) {
+    std::string str = command_line.GetSwitchValueASCII(switches::kWindowSize);
     int width, height;
-    if (ParseCommaSeparatedIntegers(str, &width, &height))
+    if (ParseCommaSeparatedIntegers(str, &width, &height)) {
       bounds->set_size(gfx::Size(width, height));
+      *show_state = ui::SHOW_STATE_NORMAL;
+    }
   }
-  if (parsed_command_line.HasSwitch(switches::kWindowPosition)) {
+  if (command_line.HasSwitch(switches::kWindowPosition)) {
     std::string str =
-        parsed_command_line.GetSwitchValueASCII(switches::kWindowPosition);
+        command_line.GetSwitchValueASCII(switches::kWindowPosition);
     int x, y;
-    if (ParseCommaSeparatedIntegers(str, &x, &y))
+    if (ParseCommaSeparatedIntegers(str, &x, &y)) {
       bounds->set_origin(gfx::Point(x, y));
+      *show_state = ui::SHOW_STATE_NORMAL;
+    }
   }
 }
 
+}  // namespace internal
+
 }  // namespace chrome

chrome/browser/ui/browser_window_state.h

@@ -14,6 +14,7 @@
 class Browser;
 
 namespace base {
+class CommandLine;
 class DictionaryValue;
 }
 
@@ -58,6 +59,17 @@ void GetSavedWindowBoundsAndShowState(const Browser* browser,
                                       gfx::Rect* bounds,
                                       ui::WindowShowState* show_state);
 
+namespace internal {
+
+// Updates window bounds and show state from the provided command-line. Part of
+// implementation of GetSavedWindowBoundsAndShowState, but exposed for testing.
+void UpdateWindowBoundsAndShowStateFromCommandLine(
+    const base::CommandLine& command_line,
+    gfx::Rect* bounds,
+    ui::WindowShowState* show_state);
+
+}  // namespace internal
+
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_UI_BROWSER_WINDOW_STATE_H_

chrome/browser/ui/browser_window_state_unittest.cc

+// Copyright 2017 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_window_state.h"
+
+#include "base/command_line.h"
+#include "chrome/common/chrome_switches.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ui_base_types.h"
+#include "ui/gfx/geometry/rect.h"
+
+namespace chrome {
+
+namespace internal {
+
+namespace {
+
+constexpr int kDefaultWidth = 1920;
+constexpr int kDefaultHeight = 1200;
+constexpr int kDefaultOffsetX = 0;
+constexpr int kDefaultOffsetY = 0;
+constexpr ui::WindowShowState kDefaultShowState = ui::SHOW_STATE_MAXIMIZED;
+
+class BrowserWindowStateTest : public testing::Test {
+ public:
+  BrowserWindowStateTest()
+      : bounds_(gfx::Point(kDefaultOffsetX, kDefaultOffsetY),
+                gfx::Size(kDefaultWidth, kDefaultHeight)),
+        show_state_(kDefaultShowState),
+        command_line_(base::CommandLine::NO_PROGRAM) {}
+
+  gfx::Rect bounds_;
+  ui::WindowShowState show_state_;
+  base::CommandLine command_line_;
+};
+
+}  // namespace
+
+TEST_F(BrowserWindowStateTest, NoCommandLineLeavesParamsIntact) {
+  UpdateWindowBoundsAndShowStateFromCommandLine(command_line_, &bounds_,
+                                                &show_state_);
+  EXPECT_EQ(bounds_.x(), kDefaultOffsetX);
+  EXPECT_EQ(bounds_.y(), kDefaultOffsetY);
+  EXPECT_EQ(bounds_.width(), kDefaultWidth);
+  EXPECT_EQ(bounds_.height(), kDefaultHeight);
+  EXPECT_EQ(show_state_, kDefaultShowState);
+}
+
+TEST_F(BrowserWindowStateTest, InvalidCommandLineLeavesParamsIntact) {
+  command_line_.AppendSwitchASCII(switches::kWindowSize, "0,abc");
+  command_line_.AppendSwitchASCII(switches::kWindowPosition, "invalid");
+
+  UpdateWindowBoundsAndShowStateFromCommandLine(command_line_, &bounds_,
+                                                &show_state_);
+  EXPECT_EQ(bounds_.x(), kDefaultOffsetX);
+  EXPECT_EQ(bounds_.y(), kDefaultOffsetY);
+  EXPECT_EQ(bounds_.width(), kDefaultWidth);
+  EXPECT_EQ(bounds_.height(), kDefaultHeight);
+  EXPECT_EQ(show_state_, kDefaultShowState);
+}
+
+TEST_F(BrowserWindowStateTest, WindowSizeOverridesShowState) {
+  command_line_.AppendSwitchASCII(switches::kWindowSize, "100,200");
+
+  UpdateWindowBoundsAndShowStateFromCommandLine(command_line_, &bounds_,
+                                                &show_state_);
+  EXPECT_EQ(bounds_.x(), kDefaultOffsetX);
+  EXPECT_EQ(bounds_.y(), kDefaultOffsetY);
+  EXPECT_EQ(bounds_.width(), 100);
+  EXPECT_EQ(bounds_.height(), 200);
+  EXPECT_EQ(show_state_, ui::SHOW_STATE_NORMAL);
+}
+
+TEST_F(BrowserWindowStateTest, WindowPositionOverridesShowState) {
+  command_line_.AppendSwitchASCII(switches::kWindowPosition, "100,200");
+
+  UpdateWindowBoundsAndShowStateFromCommandLine(command_line_, &bounds_,
+                                                &show_state_);
+  EXPECT_EQ(bounds_.x(), 100);
+  EXPECT_EQ(bounds_.y(), 200);
+  EXPECT_EQ(bounds_.width(), kDefaultWidth);
+  EXPECT_EQ(bounds_.height(), kDefaultHeight);
+  EXPECT_EQ(show_state_, ui::SHOW_STATE_NORMAL);
+}
+
+}  // namespace internal
+
+}  // namespace chrome

chrome/test/BUILD.gn

@@ -3506,6 +3506,7 @@ test("unit_tests") {
       "../browser/ui/browser_command_controller_unittest.cc",
       "../browser/ui/browser_instant_controller_unittest.cc",
       "../browser/ui/browser_unittest.cc",
+      "../browser/ui/browser_window_state_unittest.cc",
       "../browser/ui/chrome_bubble_manager_unittest.cc",
       "../browser/ui/content_settings/content_setting_bubble_model_unittest.cc",
       "../browser/ui/content_settings/content_setting_image_model_unittest.cc",