Clean up WindowSizer tests

Simplify the TestScreen to use display::ScreenBase.
Always use the global Screen::GetScreen().

The eliminates one WindowSizer constructor and the |screen_| member,
such that the code in test is similar to the code in production.

Bug: 846736
Test: unit_tests
Change-Id: I39121a9b8ae1283c4bbea6194c528450243af24e
Reviewed-on: https://chromium-review.googlesource.com/1073961Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562060}

chrome/browser/ui/ash/shell_state_client.cc

@@ -63,8 +63,8 @@ void ShellStateClient::BindAndAddClient() {
 }
 
 // static
-display::Display WindowSizer::GetDisplayForNewWindow(display::Screen* screen,
-                                                     const gfx::Rect& bounds) {
+display::Display WindowSizer::GetDisplayForNewWindow(const gfx::Rect& bounds) {
+  display::Screen* screen = display::Screen::GetScreen();
   // May be null in unit tests.
   if (g_shell_state_client) {
     // Prefer the display where the user last activated any window.

chrome/browser/ui/window_sizer/window_sizer.cc

@@ -138,24 +138,11 @@ class DefaultStateProvider : public WindowSizer::StateProvider {
 ///////////////////////////////////////////////////////////////////////////////
 // WindowSizer, public:
 
-WindowSizer::WindowSizer(
-    std::unique_ptr<StateProvider> state_provider,
-    const Browser* browser)
-    : WindowSizer(std::move(state_provider),
-                  display::Screen::GetScreen(),
-                  browser) {}
-
-WindowSizer::WindowSizer(
-    std::unique_ptr<StateProvider> state_provider,
-    display::Screen* screen,
-    const Browser* browser)
-    : state_provider_(std::move(state_provider)),
-      screen_(screen),
-      browser_(browser) {
-  DCHECK(screen_);
-}
+WindowSizer::WindowSizer(std::unique_ptr<StateProvider> state_provider,
+                         const Browser* browser)
+    : state_provider_(std::move(state_provider)), browser_(browser) {}
 
-WindowSizer::~WindowSizer() {}
+WindowSizer::~WindowSizer() = default;
 
 // static
 void WindowSizer::GetBrowserWindowBoundsAndShowState(
@@ -201,7 +188,7 @@ void WindowSizer::DetermineWindowBoundsAndShowState(
 
     // No saved placement, figure out some sensible default size based on
     // the user's screen size.
-    GetDefaultWindowBounds(GetTargetDisplay(gfx::Rect()), bounds);
+    GetDefaultWindowBounds(GetDisplayForNewWindow(), bounds);
     return;
   }
 
@@ -211,7 +198,8 @@ void WindowSizer::DetermineWindowBoundsAndShowState(
   // of the anchor window. Note: AdjustBoundsToBeVisibleOnMonitorContaining
   // does not exactly what we want: It makes only sure that "a minimal part"
   // is visible on the screen.
-  gfx::Rect work_area = screen_->GetDisplayMatching(*bounds).work_area();
+  gfx::Rect work_area =
+      display::Screen::GetScreen()->GetDisplayMatching(*bounds).work_area();
   // Resize so that it fits.
   bounds->AdjustToFit(work_area);
 }
@@ -225,9 +213,9 @@ bool WindowSizer::GetLastActiveWindowBounds(
       !state_provider_->GetLastActiveWindowState(bounds, show_state))
     return false;
   bounds->Offset(kWindowTilePixels, kWindowTilePixels);
-  AdjustBoundsToBeVisibleOnDisplay(screen_->GetDisplayMatching(*bounds),
-                                   gfx::Rect(),
-                                   bounds);
+  AdjustBoundsToBeVisibleOnDisplay(
+      display::Screen::GetScreen()->GetDisplayMatching(*bounds), gfx::Rect(),
+      bounds);
   return true;
 }
 
@@ -241,9 +229,8 @@ bool WindowSizer::GetSavedWindowBounds(gfx::Rect* bounds,
                                            &saved_work_area,
                                            show_state))
     return false;
-  AdjustBoundsToBeVisibleOnDisplay(GetTargetDisplay(*bounds),
-                                   saved_work_area,
-                                   bounds);
+  AdjustBoundsToBeVisibleOnDisplay(GetDisplayForNewWindow(*bounds),
+                                   saved_work_area, bounds);
   return true;
 }
 
@@ -265,7 +252,8 @@ void WindowSizer::GetDefaultWindowBounds(const display::Display& display,
 #if !defined(OS_MACOSX)
   // For wider aspect ratio displays at higher resolutions, we might size the
   // window narrower to allow two windows to easily be placed side-by-side.
-  gfx::Rect screen_size = screen_->GetPrimaryDisplay().bounds();
+  gfx::Rect screen_size =
+      display::Screen::GetScreen()->GetPrimaryDisplay().bounds();
   double width_to_height =
     static_cast<double>(screen_size.width()) / screen_size.height();
 
@@ -358,10 +346,6 @@ void WindowSizer::AdjustBoundsToBeVisibleOnDisplay(
 #endif  // defined(OS_MACOSX)
 }
 
-display::Display WindowSizer::GetTargetDisplay(const gfx::Rect& bounds) const {
-  return GetDisplayForNewWindow(screen_, bounds);
-}
-
 ui::WindowShowState WindowSizer::GetWindowDefaultShowState() const {
   if (!browser_)
     return ui::SHOW_STATE_DEFAULT;
@@ -385,8 +369,7 @@ ui::WindowShowState WindowSizer::GetWindowDefaultShowState() const {
 #if !defined(OS_CHROMEOS)
 // Chrome OS has an implementation in //chrome/browser/ui/ash.
 // static
-display::Display WindowSizer::GetDisplayForNewWindow(display::Screen* screen,
-                                                     const gfx::Rect& bounds) {
-  return screen->GetDisplayMatching(bounds);
+display::Display WindowSizer::GetDisplayForNewWindow(const gfx::Rect& bounds) {
+  return display::Screen::GetScreen()->GetDisplayMatching(bounds);
 }
 #endif  // defined(OS_CHROMEOS)

chrome/browser/ui/window_sizer/window_sizer.h

@@ -113,11 +113,6 @@ class WindowSizer {
   WindowSizer(std::unique_ptr<StateProvider> state_provider,
               const Browser* browser);
 
-  // As above, but uses the supplied |screen|. Used only for testing.
-  WindowSizer(std::unique_ptr<StateProvider> state_provider,
-              display::Screen* screen,
-              const Browser* browser);
-
   virtual ~WindowSizer();
 
   // The edge of the screen to check for out-of-bounds.
@@ -157,11 +152,6 @@ class WindowSizer {
                                         const gfx::Rect& saved_work_area,
                                         gfx::Rect* bounds) const;
 
-  // Determine the target display for a new window based on
-  // |bounds|. On ash environment, this returns the display containing
-  // ash's the target root window.
-  display::Display GetTargetDisplay(const gfx::Rect& bounds) const;
-
 #if defined(OS_CHROMEOS)
   // Ash specific logic for window placement. Returns true if |bounds| and
   // |show_state| have been fully determined, otherwise returns false (but
@@ -189,12 +179,11 @@ class WindowSizer {
 
   // Returns the target display for a new window with |bounds| in screen
   // coordinates.
-  static display::Display GetDisplayForNewWindow(display::Screen* screen,
-                                                 const gfx::Rect& bounds);
+  static display::Display GetDisplayForNewWindow(
+      const gfx::Rect& bounds = gfx::Rect());
 
   // Providers for persistent storage and monitor metrics.
   std::unique_ptr<StateProvider> state_provider_;
-  display::Screen* screen_;  // not owned.
 
   // Note that this browser handle might be NULL.
   const Browser* const browser_;

chrome/browser/ui/window_sizer/window_sizer_ash.cc

@@ -54,7 +54,7 @@ bool WindowSizer::GetBrowserBoundsAsh(gfx::Rect* bounds,
       // For trusted popups (v1 apps and system windows), do not use the last
       // active window bounds, only use saved or default bounds.
       if (!GetSavedWindowBounds(bounds, show_state))
-        *bounds = GetDefaultWindowBoundsAsh(GetTargetDisplay(gfx::Rect()));
+        *bounds = GetDefaultWindowBoundsAsh(GetDisplayForNewWindow());
       determined = true;
     } else if (state_provider_) {
       // In Ash, prioritize the last saved |show_state|. If you have questions
@@ -66,7 +66,8 @@ bool WindowSizer::GetBrowserBoundsAsh(gfx::Rect* bounds,
   }
 
   if (browser_->is_type_tabbed() && *show_state == ui::SHOW_STATE_DEFAULT) {
-    display::Display display = screen_->GetDisplayMatching(*bounds);
+    display::Display display =
+        display::Screen::GetScreen()->GetDisplayMatching(*bounds);
     gfx::Rect work_area = display.work_area();
     bounds->AdjustToFit(work_area);
     if (*bounds == work_area) {
@@ -93,13 +94,14 @@ void WindowSizer::GetTabbedBrowserBoundsAsh(
   const ui::WindowShowState passed_show_state = *show_state;
 
   bool is_saved_bounds = GetSavedWindowBounds(bounds_in_screen, show_state);
-  display::Display display = GetTargetDisplay(*bounds_in_screen);
+  display::Display display = GetDisplayForNewWindow(*bounds_in_screen);
   if (!is_saved_bounds)
     *bounds_in_screen = GetDefaultWindowBoundsAsh(display);
 
   if (browser_->is_session_restore()) {
     // Respect display for saved bounds during session restore.
-    display = screen_->GetDisplayMatching(*bounds_in_screen);
+    display =
+        display::Screen::GetScreen()->GetDisplayMatching(*bounds_in_screen);
   } else if (BrowserList::GetInstance()->empty() && !is_saved_bounds &&
              (ShouldForceMaximizeOnFirstRun() ||
               display.work_area().width() <= kForceMaximizeWidthLimit)) {

chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc

@@ -67,6 +67,7 @@ constexpr int kForceMaximizeWidthLimit = 1366;
 using util = WindowSizerTestUtil;
 const int kDesktopBorderSize = WindowSizer::kDesktopBorderSize;
 const int kMaximumWindowWidth = WindowSizer::kMaximumWindowWidth;
+const int kWindowTilePixels = WindowSizer::kWindowTilePixels;
 
 std::unique_ptr<Browser> CreateTestBrowser(aura::Window* window,
                                            const gfx::Rect& bounds,

chrome/browser/ui/window_sizer/window_sizer_common_unittest.cc

@@ -16,6 +16,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
+#include "ui/display/screen_base.h"
 
 #if defined(USE_AURA)
 #include "ui/aura/window.h"
@@ -25,8 +26,7 @@ namespace {
 
 using util = WindowSizerTestUtil;
 
-// TODO(rjkroege): Use the common TestScreen.
-class TestScreen : public display::Screen {
+class TestScreen : public display::ScreenBase {
  public:
   TestScreen() : previous_screen_(display::Screen::GetScreen()) {
     display::Screen::SetScreenInstance(this);
@@ -35,88 +35,18 @@ class TestScreen : public display::Screen {
     display::Screen::SetScreenInstance(previous_screen_);
   }
 
-  // Sets the index of the display returned from GetDisplayNearestWindow().
-  // Only used on aura.
-  void set_index_of_display_nearest_window(int index) {
-    index_of_display_nearest_window_ = index;
-  }
-
-  // Overridden from display::Screen:
-  gfx::Point GetCursorScreenPoint() override {
-    NOTREACHED();
-    return gfx::Point();
-  }
-
-  bool IsWindowUnderCursor(gfx::NativeWindow window) override {
-    NOTIMPLEMENTED();
-    return false;
-  }
-
-  gfx::NativeWindow GetWindowAtScreenPoint(const gfx::Point& point) override {
-    NOTREACHED();
-    return NULL;
-  }
-
-  int GetNumDisplays() const override { return displays_.size(); }
-
-  const std::vector<display::Display>& GetAllDisplays() const override {
-    return displays_;
-  }
-
-  display::Display GetDisplayNearestWindow(
-      gfx::NativeWindow window) const override {
-#if defined(USE_AURA)
-    return displays_[index_of_display_nearest_window_];
-#else
-    NOTREACHED();
-    return display::Display();
-#endif
-  }
-
-  display::Display GetDisplayNearestPoint(
-      const gfx::Point& point) const override {
-    NOTREACHED();
-    return display::Display();
-  }
-
-  display::Display GetDisplayMatching(
-      const gfx::Rect& match_rect) const override {
-    int max_area = 0;
-    size_t max_area_index = 0;
-
-    for (size_t i = 0; i < displays_.size(); ++i) {
-      gfx::Rect overlap = displays_[i].bounds();
-      overlap.Intersect(match_rect);
-      int area = overlap.width() * overlap.height();
-      if (area > max_area) {
-        max_area = area;
-        max_area_index = i;
-      }
-    }
-    return displays_[max_area_index];
-  }
-
-  display::Display GetPrimaryDisplay() const override { return displays_[0]; }
-
-  void AddObserver(display::DisplayObserver* observer) override {
-    NOTREACHED();
-  }
-
-  void RemoveObserver(display::DisplayObserver* observer) override {
-    NOTREACHED();
-  }
-
   void AddDisplay(const gfx::Rect& bounds,
                   const gfx::Rect& work_area) {
-    display::Display display(displays_.size(), bounds);
+    display::Display display(display_list().displays().size(), bounds);
     display.set_work_area(work_area);
-    displays_.push_back(display);
+    display_list().AddDisplay(display,
+                              display_list().displays().empty()
+                                  ? display::DisplayList::Type::PRIMARY
+                                  : display::DisplayList::Type::NOT_PRIMARY);
   }
 
  private:
   display::Screen* previous_screen_;
-  size_t index_of_display_nearest_window_ = 0u;
-  std::vector<display::Display> displays_;
 
   DISALLOW_COPY_AND_ASSIGN(TestScreen);
 };
@@ -169,8 +99,6 @@ bool TestStateProvider::GetLastActiveWindowState(
   return has_last_active_data_;
 }
 
-int kWindowTilePixels = WindowSizer::kWindowTilePixels;
-
 // static
 void WindowSizerTestUtil::GetWindowBoundsAndShowState(
     const gfx::Rect& monitor1_bounds,
@@ -191,14 +119,13 @@ void WindowSizerTestUtil::GetWindowBoundsAndShowState(
   test_screen.AddDisplay(monitor1_bounds, monitor1_work_area);
   if (!monitor2_bounds.IsEmpty())
     test_screen.AddDisplay(monitor2_bounds, monitor2_bounds);
-  test_screen.set_index_of_display_nearest_window(display_index);
   std::unique_ptr<TestStateProvider> sp(new TestStateProvider);
   if (source == PERSISTED || source == BOTH)
     sp->SetPersistentState(bounds, work_area, show_state_persisted, true);
   if (source == LAST_ACTIVE || source == BOTH)
     sp->SetLastActiveState(bounds, show_state_last, true);
 
-  WindowSizer sizer(std::move(sp), &test_screen, browser);
+  WindowSizer sizer(std::move(sp), browser);
   sizer.DetermineWindowBoundsAndShowState(passed_in,
                                           out_bounds,
                                           out_show_state);
@@ -220,7 +147,7 @@ ui::WindowShowState WindowSizerTestUtil::GetWindowShowState(
   if (source == LAST_ACTIVE || source == BOTH)
     sp->SetLastActiveState(bounds, show_state_last, true);
 
-  WindowSizer sizer(std::move(sp), &test_screen, browser);
+  WindowSizer sizer(std::move(sp), browser);
 
   ui::WindowShowState out_show_state = ui::SHOW_STATE_DEFAULT;
   gfx::Rect out_bounds;

chrome/browser/ui/window_sizer/window_sizer_common_unittest.h

@@ -46,8 +46,6 @@ static const gfx::Rect taskbar_top_work_area(0, 34, 1024, 734);
 static const gfx::Rect taskbar_left_work_area(107, 0, 917, 768);
 static const gfx::Rect taskbar_right_work_area(0, 0, 917, 768);
 
-extern int kWindowTilePixels;
-
 // Testing implementation of WindowSizer::StateProvider that we use to fake
 // persistent storage and existing windows.
 class TestStateProvider : public WindowSizer::StateProvider {

chrome/browser/ui/window_sizer/window_sizer_unittest.cc

@@ -10,6 +10,7 @@
 
 namespace {
 using util = WindowSizerTestUtil;
+const int kWindowTilePixels = WindowSizer::kWindowTilePixels;
 }
 
 // Test that the window is sized appropriately for the first run experience