Implemented ForceMaximizeBrowserWindowOnFirstRun policy, added unit test and browser test.

chrome/browser/chromeos/policy/force_maximize_on_first_run_chromeos_browsertest.cc

+// Copyright 2015 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 <string>

[bartfab (slow), 2015/04/01 14:22:42]

Nit: Add a blank line above.

[peletskyi, 2015/04/01 18:55:46]

Done.

+
+#include "ash/display/display_manager.h"
+#include "ash/shell.h"
+#include "ash/test/display_manager_test_api.h"
+#include "ash/wm/window_positioner.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/browser/chromeos/policy/login_policy_test_base.h"
+#include "chrome/browser/profiles/profile_manager.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/host_desktop.h"
+#include "policy/policy_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace policy {
+
+class ForceMaximizeOnFirstRunTest : public LoginPolicyTestBase {
+ protected:
+  ForceMaximizeOnFirstRunTest() : LoginPolicyTestBase(){};

[bartfab (slow), 2015/04/01 14:22:42]

Nit: s/{};/ {}/

[peletskyi, 2015/04/01 18:55:45]

Done.

+
+  std::string GetMandatoryPolicies() const override {
+    return "\"" + std::string(key::kForceMaximizeOnFirstRun) + "\": true";
+  }
+
+  void SetUpResolution() {
+    // Set width of the screen wider than limit, after which the browser window
+    // is *NOT* maximized by default.

[bartfab (slow), 2015/04/01 14:22:42]

Nit: The sentence is a bit hard to parse. How about something like "Set a screen
resolution for which the first browser window will not be maximized by default?"

[peletskyi, 2015/04/01 18:55:46]

Done.

+    const int width =
+        ash::WindowPositioner::GetForceMaximizedWidthLimit() + 100;
+    // Set resolution to 1466x300.
+    const std::string resolution = base::IntToString(width) + "x300";
+    ash::DisplayManager* display_manager =

[bartfab (slow), 2015/04/01 14:22:42]

Nit: const pointer.

[peletskyi, 2015/04/01 18:55:46]

ash::test::DisplayManagerTestApi takes non-const pointer.

[bartfab (slow), 2015/04/02 08:44:28]

It takes a pointer to non-const, but it does not care whether the pointer itself
is const or not.

[peletskyi, 2015/04/02 12:53:28]

Done.

+        ash::Shell::GetInstance()->display_manager();
+    ash::test::DisplayManagerTestApi display_manager_test_api(display_manager);
+    display_manager_test_api.UpdateDisplay(resolution);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ForceMaximizeOnFirstRunTest);

[bartfab (slow), 2015/04/01 14:22:42]

Nit: #include "base/macros.h"

[peletskyi, 2015/04/01 18:55:46]

Done.

+};
+
+IN_PROC_BROWSER_TEST_F(ForceMaximizeOnFirstRunTest, PRE_TwoRuns) {
+  SetUpResolution();
+  SkipToLoginScreen();
+  LogIn(kAccountId, kAccountPassword);
+
+  // Check that the first browser window is maximized.
+  const BrowserList* browser_list =

[bartfab (slow), 2015/04/01 14:22:42]

Nit: const pointer.

[peletskyi, 2015/04/01 18:55:46]

Already const

[bartfab (slow), 2015/04/02 08:44:28]

This is a pointer to const. Make it a const pointer as well:

const BrowserList* const browser_list =

[peletskyi, 2015/04/02 12:53:29]

Done.

+      BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
+  EXPECT_EQ(1U, browser_list->size());
+  Browser* browser = browser_list->get(0);

[bartfab (slow), 2015/04/01 14:22:42]

Nit: const pointer.

[peletskyi, 2015/04/01 18:55:45]

Done.

[bartfab (slow), 2015/04/02 08:44:28]

Nice to see that it can be a pointer to coinst. Please also make it a const
pointer :).

[peletskyi, 2015/04/02 12:53:28]

Done.

+  ASSERT_TRUE(browser);
+  EXPECT_TRUE(browser->window()->IsMaximized());
+
+  browser->window()->Restore();

[bartfab (slow), 2015/04/01 14:22:41]

Nit: Add a comment that you need to un-maximize the window here as its state
will be carried forward to the next window you open.

[peletskyi, 2015/04/01 18:55:46]

Done.

+  EXPECT_FALSE(browser->window()->IsMaximized());
+
+  // Create the second window and check if it is not affected by policy.

[bartfab (slow), 2015/04/01 14:22:42]

Nit 1: a/the/a/
Nit 2: s/if/that/

[peletskyi, 2015/04/01 18:55:46]

Done.

+  Browser* browser1 = CreateBrowser(ProfileManager::GetActiveUserProfile());

[bartfab (slow), 2015/04/01 14:22:41]

1: As with chrome_shell_delegate.cc, the documentation for this method says that
you should "avoid using it at all cost."
2: Nit: const pointer. Alternatively, make |browser| not be a const pointer and
simply reset |browser| to the new window here.

[bartfab (slow), 2015/04/02 08:44:28]

1: This was not not addressed.
2: Please make |browser1| a const pointer to const.

[peletskyi, 2015/04/02 12:53:28]

Done.

+  ASSERT_TRUE(browser);
+  EXPECT_FALSE(browser1->window()->IsMaximized());
+}
+
+IN_PROC_BROWSER_TEST_F(ForceMaximizeOnFirstRunTest, TwoRuns) {
+  SetUpResolution();
+  content::WindowedNotificationObserver(

[bartfab (slow), 2015/04/01 14:22:41]

Nit: #include "content/public/test/test_utils.h"

[peletskyi, 2015/04/01 18:55:45]

Done.

+      chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,

[bartfab (slow), 2015/04/01 14:22:42]

Nit: #include "chrome/browser/chrome_notification_types.h"

[peletskyi, 2015/04/01 18:55:46]

Done.

+      content::NotificationService::AllSources()).Wait();

[bartfab (slow), 2015/04/01 14:22:42]

Nit: #include "content/public/browser/notification_service.h"

[peletskyi, 2015/04/01 18:55:46]

Done.

+

[bartfab (slow), 2015/04/01 14:22:42]

Nit: For consistency with the first test, remove the blank line.

[peletskyi, 2015/04/01 18:55:46]

Done.

+  LogIn(kAccountId, kAccountPassword);
+
+  Browser* browser = CreateBrowser(ProfileManager::GetActiveUserProfile());

[bartfab (slow), 2015/04/01 14:22:42]

Nit: const pointer.

[peletskyi, 2015/04/01 18:55:46]

Done.

[bartfab (slow), 2015/04/02 08:44:28]

Please make this a const pointer to const.

[peletskyi, 2015/04/02 12:53:28]

Done.

+  ASSERT_TRUE(browser);
+  EXPECT_FALSE(browser->window()->IsMaximized());
+}
+
+class ForceMaximizetPolicyFalseTest : public ForceMaximizeOnFirstRunTest {
+ protected:
+  ForceMaximizetPolicyFalseTest() : ForceMaximizeOnFirstRunTest(){};

[bartfab (slow), 2015/04/01 14:22:42]

Nit: s/{};/ {}/

[peletskyi, 2015/04/01 18:55:46]

Done.

+  std::string GetMandatoryPolicies() const override {

[bartfab (slow), 2015/04/01 14:22:42]

Nit: Add a blank line above.

[peletskyi, 2015/04/01 18:55:46]

Done.

+    return "\"" + std::string(key::kForceMaximizeOnFirstRun) + "\": false";
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ForceMaximizetPolicyFalseTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ForceMaximizetPolicyFalseTest, GeneralFirstRun) {
+  SetUpResolution();
+

[bartfab (slow), 2015/04/01 14:22:42]

Nit: For consistency with the first test, remove the blank line.

[peletskyi, 2015/04/02 12:53:28]

Done.

+  SkipToLoginScreen();
+  LogIn(kAccountId, kAccountPassword);
+
+  const BrowserList* browser_list =

[bartfab (slow), 2015/04/01 14:22:42]

Nit: Const pointer.

[peletskyi, 2015/04/01 18:55:46]

Done.

[bartfab (slow), 2015/04/02 08:44:28]

Please make this a const pointer to const.

[peletskyi, 2015/04/02 12:53:29]

Done.

+      BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
+  EXPECT_EQ(1U, browser_list->size());
+  const Browser* browser = browser_list->get(0);

[bartfab (slow), 2015/04/01 14:22:42]

Nit: Const pointer.

[peletskyi, 2015/04/01 18:55:46]

Done.

[bartfab (slow), 2015/04/02 08:44:28]

Please make this a const pointer to const.

[peletskyi, 2015/04/02 12:53:28]

Done.

+  ASSERT_TRUE(browser);
+  EXPECT_FALSE(browser->window()->IsMaximized());
+}
+
+}  // namespace policy