ScreenWin Testability and Restructuring

ui/gfx/screen_win.cc

Index: ui/gfx/screen_win.cc
diff --git a/ui/gfx/screen_win.cc b/ui/gfx/screen_win.cc
index 3199f7acaacdd2b15c3dca37d09014dc418c2d00..5b7f8377b2de31cb02758fa42a5377bffcf038c9 100644
--- a/ui/gfx/screen_win.cc
+++ b/ui/gfx/screen_win.cc
@@ -5,97 +5,75 @@
 #include "ui/gfx/screen_win.h"
 
 #include <windows.h>
-#include <stdint.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/hash.h"
-#include "base/logging.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/win/win_util.h"
 #include "ui/gfx/display.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/win/display_info.h"
 #include "ui/gfx/win/dpi.h"
+#include "ui/gfx/win/screen_win_display.h"
 
 namespace {
 
-MONITORINFOEX GetMonitorInfoForMonitor(HMONITOR monitor) {
-  MONITORINFOEX monitor_info;
-  ZeroMemory(&monitor_info, sizeof(MONITORINFOEX));
-  monitor_info.cbSize = sizeof(monitor_info);
-  GetMonitorInfo(monitor, &monitor_info);
-  return monitor_info;
+std::vector<gfx::win::ScreenWinDisplay> DisplayInfosToScreenWinDisplays(
+    const std::vector<gfx::win::DisplayInfo>& display_infos) {
+  std::vector<gfx::win::ScreenWinDisplay> screen_win_displays;
+  for (const auto& display_info : display_infos)
+    screen_win_displays.push_back(gfx::win::ScreenWinDisplay(display_info));
+
+  return screen_win_displays;
 }
 
-gfx::Display GetDisplay(const MONITORINFOEX& monitor_info) {
-  int64_t id =
-      static_cast<int64_t>(base::Hash(base::WideToUTF8(monitor_info.szDevice)));
-  gfx::Rect bounds = gfx::Rect(monitor_info.rcMonitor);
-  gfx::Display display(id);
-  display.set_bounds(gfx::win::ScreenToDIPRect(bounds));
-  display.set_work_area(
-      gfx::win::ScreenToDIPRect(gfx::Rect(monitor_info.rcWork)));
-  display.SetScaleAndBounds(gfx::GetDPIScale(), bounds);
-
-  DEVMODE mode;
-  memset(&mode, 0, sizeof(DEVMODE));
-  mode.dmSize = sizeof(DEVMODE);
-  mode.dmDriverExtra = 0;
-  if (EnumDisplaySettings(monitor_info.szDevice,
-                          ENUM_CURRENT_SETTINGS,
-                          &mode)) {
-    switch (mode.dmDisplayOrientation) {
-    case DMDO_DEFAULT:
-      display.set_rotation(gfx::Display::ROTATE_0);
-      break;
-    case DMDO_90:
-      display.set_rotation(gfx::Display::ROTATE_90);
-      break;
-    case DMDO_180:
-      display.set_rotation(gfx::Display::ROTATE_180);
-      break;
-    case DMDO_270:
-      display.set_rotation(gfx::Display::ROTATE_270);
-      break;
-    default:
-      NOTREACHED();
-    }
-  }
+std::vector<gfx::Display> ScreenWinDisplaysToDisplays(
+    const std::vector<gfx::win::ScreenWinDisplay>& screen_win_displays) {
+  std::vector<gfx::Display> displays;
+  for (const auto& screen_win_display : screen_win_displays)
+    displays.push_back(screen_win_display.display());
 
-  return display;
+  return displays;
+}
+
+MONITORINFOEX MonitorInfoFromHMONITOR(HMONITOR monitor) {
+  MONITORINFOEX monitor_info;
+  ::ZeroMemory(&monitor_info, sizeof(monitor_info));
+  monitor_info.cbSize = sizeof(monitor_info);
+  ::GetMonitorInfo(monitor, &monitor_info);
+  return monitor_info;
 }
 
 BOOL CALLBACK EnumMonitorCallback(HMONITOR monitor,
                                   HDC hdc,
                                   LPRECT rect,
                                   LPARAM data) {
-  std::vector<gfx::Display>* all_displays =
-      reinterpret_cast<std::vector<gfx::Display>*>(data);
-  DCHECK(all_displays);
-
-  MONITORINFOEX monitor_info = GetMonitorInfoForMonitor(monitor);
-  gfx::Display display = GetDisplay(monitor_info);
-  all_displays->push_back(display);
+  std::vector<gfx::win::DisplayInfo>* display_infos =
+      reinterpret_cast<std::vector<gfx::win::DisplayInfo>*>(data);
+  DCHECK(display_infos);
+  display_infos->push_back(
+      gfx::win::DisplayInfo(MonitorInfoFromHMONITOR(monitor),
+                            gfx::GetDPIScale()));
   return TRUE;
 }
 
-std::vector<gfx::Display> GetDisplays() {
-  std::vector<gfx::Display> displays;
+std::vector<gfx::win::DisplayInfo> GetDisplayInfosFromSystem() {
+  std::vector<gfx::win::DisplayInfo> display_infos;
   EnumDisplayMonitors(NULL, NULL, EnumMonitorCallback,
-                      reinterpret_cast<LPARAM>(&displays));
-  return displays;
+                      reinterpret_cast<LPARAM>(&display_infos));
+  DCHECK_EQ(static_cast<size_t>(::GetSystemMetrics(SM_CMONITORS)),
+            display_infos.size());
+  return display_infos;
 }
 
 }  // namespace
 
 namespace gfx {
 
-ScreenWin::ScreenWin()
-    : singleton_hwnd_observer_(new SingletonHwndObserver(
-          base::Bind(&ScreenWin::OnWndProc, base::Unretained(this)))),
-      displays_(GetDisplays()) {
+ScreenWin::ScreenWin() {
+  Initialize();
 }
 
-ScreenWin::~ScreenWin() {}
+ScreenWin::~ScreenWin() = default;
 
 HWND ScreenWin::GetHWNDFromNativeView(NativeView window) const {
   NOTREACHED();
@@ -109,14 +87,15 @@ NativeWindow ScreenWin::GetNativeWindowFromHWND(HWND hwnd) const {
 
 gfx::Point ScreenWin::GetCursorScreenPoint() {
   POINT pt;
-  GetCursorPos(&pt);
+  ::GetCursorPos(&pt);
   gfx::Point cursor_pos_pixels(pt);
   return gfx::win::ScreenToDIPPoint(cursor_pos_pixels);
 }
 
 gfx::NativeWindow ScreenWin::GetWindowUnderCursor() {
   POINT cursor_loc;
-  HWND hwnd = GetCursorPos(&cursor_loc) ? WindowFromPoint(cursor_loc) : NULL;
+  HWND hwnd =
+      ::GetCursorPos(&cursor_loc) ? ::WindowFromPoint(cursor_loc) : NULL;
   return GetNativeWindowFromHWND(hwnd);
 }
 
@@ -126,11 +105,11 @@ gfx::NativeWindow ScreenWin::GetWindowAtScreenPoint(const gfx::Point& point) {
 }
 
 int ScreenWin::GetNumDisplays() const {
-  return GetSystemMetrics(SM_CMONITORS);
+  return static_cast<int>(screen_win_displays_.size());
 }
 
 std::vector<gfx::Display> ScreenWin::GetAllDisplays() const {
-  return displays_;
+  return ScreenWinDisplaysToDisplays(screen_win_displays_);
 }
 
 gfx::Display ScreenWin::GetDisplayNearestWindow(gfx::NativeView window) const {
@@ -141,45 +120,26 @@ gfx::Display ScreenWin::GetDisplayNearestWindow(gfx::NativeView window) const {
     // scaling factor.
     return GetPrimaryDisplay();
   }
-
-  MONITORINFOEX monitor_info;
-  monitor_info.cbSize = sizeof(monitor_info);
-  GetMonitorInfo(MonitorFromWindow(window_hwnd, MONITOR_DEFAULTTONEAREST),
-                 &monitor_info);
-  return GetDisplay(monitor_info);
+  gfx::win::ScreenWinDisplay screen_win_display =
+      GetScreenWinDisplayNearestHWND(window_hwnd);
+  return screen_win_display.display();
 }
 
 gfx::Display ScreenWin::GetDisplayNearestPoint(const gfx::Point& point) const {
-  gfx::Point point_in_pixels = gfx::win::DIPToScreenPoint(point);
-  POINT initial_loc = { point_in_pixels.x(), point_in_pixels.y() };
-  HMONITOR monitor = MonitorFromPoint(initial_loc, MONITOR_DEFAULTTONEAREST);
-  MONITORINFOEX mi;
-  ZeroMemory(&mi, sizeof(MONITORINFOEX));
-  mi.cbSize = sizeof(mi);
-  if (monitor && GetMonitorInfo(monitor, &mi)) {
-    return GetDisplay(mi);
-  }
-  return gfx::Display();
+  gfx::Point screen_point(gfx::win::DIPToScreenPoint(point));
+  gfx::win::ScreenWinDisplay screen_win_display =
+      GetScreenWinDisplayNearestScreenPoint(screen_point);
+  return screen_win_display.display();
 }
 
 gfx::Display ScreenWin::GetDisplayMatching(const gfx::Rect& match_rect) const {
-  RECT other_bounds_rect = match_rect.ToRECT();
-  MONITORINFOEX monitor_info = GetMonitorInfoForMonitor(MonitorFromRect(
-      &other_bounds_rect, MONITOR_DEFAULTTONEAREST));
-  return GetDisplay(monitor_info);
+  gfx::win::ScreenWinDisplay screen_win_display =
+      GetScreenWinDisplayNearestScreenRect(match_rect);
+  return screen_win_display.display();
 }
 
 gfx::Display ScreenWin::GetPrimaryDisplay() const {
-  MONITORINFOEX mi = GetMonitorInfoForMonitor(
-      MonitorFromWindow(NULL, MONITOR_DEFAULTTOPRIMARY));
-  gfx::Display display = GetDisplay(mi);
-  // TODO(kevers|girard): Test if these checks can be reintroduced for high-DIP
-  // once more of the app is DIP-aware.
-  if (GetDPIScale() == 1.0) {
-    DCHECK_EQ(GetSystemMetrics(SM_CXSCREEN), display.size().width());
-    DCHECK_EQ(GetSystemMetrics(SM_CYSCREEN), display.size().height());
-  }
-  return display;
+  return GetPrimaryScreenWinDisplay().display();
 }
 
 void ScreenWin::AddObserver(DisplayObserver* observer) {
@@ -190,6 +150,41 @@ void ScreenWin::RemoveObserver(DisplayObserver* observer) {
   change_notifier_.RemoveObserver(observer);
 }
 
+void ScreenWin::UpdateFromDisplayInfos(
+    const std::vector<gfx::win::DisplayInfo>& display_infos) {
+  screen_win_displays_ = DisplayInfosToScreenWinDisplays(display_infos);
+}
+
+void ScreenWin::Initialize() {
+  singleton_hwnd_observer_.reset(
+      new gfx::SingletonHwndObserver(
+          base::Bind(&ScreenWin::OnWndProc, base::Unretained(this))));
+  UpdateFromDisplayInfos(GetDisplayInfosFromSystem());
+}
+
+MONITORINFOEX ScreenWin::MonitorInfoFromScreenPoint(
+    const gfx::Point& screen_point) const {
+  POINT initial_loc = { screen_point.x(), screen_point.y() };
+  return MonitorInfoFromHMONITOR(::MonitorFromPoint(initial_loc,
+                                                    MONITOR_DEFAULTTONEAREST));
+}
+
+MONITORINFOEX ScreenWin::MonitorInfoFromScreenRect(const gfx::Rect& screen_rect)
+    const {
+  RECT win_rect = screen_rect.ToRECT();
+  return MonitorInfoFromHMONITOR(::MonitorFromRect(&win_rect,
+                                                   MONITOR_DEFAULTTONEAREST));
+}
+
+MONITORINFOEX ScreenWin::MonitorInfoFromWindow(HWND hwnd,
+                                               DWORD default_options) const {
+  return MonitorInfoFromHMONITOR(::MonitorFromWindow(hwnd, default_options));
+}
+
+HWND ScreenWin::GetRootWindow(HWND hwnd) const {
+  return ::GetAncestor(hwnd, GA_ROOT);
+}
+
 void ScreenWin::OnWndProc(HWND hwnd,
                           UINT message,
                           WPARAM wparam,
@@ -197,20 +192,49 @@ void ScreenWin::OnWndProc(HWND hwnd,
   if (message != WM_DISPLAYCHANGE)
     return;
 
-  std::vector<gfx::Display> old_displays = displays_;
-  displays_ = GetDisplays();
+  std::vector<gfx::Display> old_displays = GetAllDisplays();
+  UpdateFromDisplayInfos(GetDisplayInfosFromSystem());
+  change_notifier_.NotifyDisplaysChanged(old_displays, GetAllDisplays());
+}
 
-  change_notifier_.NotifyDisplaysChanged(old_displays, displays_);
+gfx::win::ScreenWinDisplay ScreenWin::GetScreenWinDisplayNearestHWND(HWND hwnd)
+    const {
+  return GetScreenWinDisplay(MonitorInfoFromWindow(hwnd,
+                                                   MONITOR_DEFAULTTONEAREST));
 }
 
-// static
-std::vector<gfx::Display> ScreenWin::GetDisplaysForMonitorInfos(
-    const std::vector<MONITORINFOEX>& monitor_infos) {
-  std::vector<gfx::Display> displays;
-  for (const MONITORINFOEX& monitor_info : monitor_infos)
-    displays.push_back(GetDisplay(monitor_info));
+gfx::win::ScreenWinDisplay ScreenWin::GetScreenWinDisplayNearestScreenRect(
+    const Rect& screen_rect) const {
+  return GetScreenWinDisplay(MonitorInfoFromScreenRect(screen_rect));
+}
 
-  return displays;
+gfx::win::ScreenWinDisplay ScreenWin::GetScreenWinDisplayNearestScreenPoint(
+    const Point& screen_point) const {
+  return GetScreenWinDisplay(MonitorInfoFromScreenPoint(screen_point));
+}
+
+gfx::win::ScreenWinDisplay ScreenWin::GetPrimaryScreenWinDisplay() const {
+  MONITORINFOEX monitor_info = MonitorInfoFromWindow(NULL,

[sky, 2016/02/03 21:47:37]

nullptr

[robliao, 2016/02/03 22:47:20]

Done.

+                                                     MONITOR_DEFAULTTOPRIMARY);
+  gfx::win::ScreenWinDisplay screen_win_display =
+      GetScreenWinDisplay(monitor_info);
+  gfx::Display display = screen_win_display.display();
+  // The Windows primary monitor is defined to have an origin of (0, 0).
+  DCHECK_EQ(0, display.bounds().origin().x());
+  DCHECK_EQ(0, display.bounds().origin().y());
+  return screen_win_display;
+}
+
+gfx::win::ScreenWinDisplay ScreenWin::GetScreenWinDisplay(
+    const MONITORINFOEX& monitor_info) const {
+  int64_t id = gfx::win::DisplayInfo::HashDeviceName(monitor_info.szDevice);
+  for (const auto& screen_win_display : screen_win_displays_) {
+    if (screen_win_display.display().id() == id)
+      return screen_win_display;
+  }
+  // When the system isn't initialized, it means we're likely under a test.

[sky, 2016/02/03 21:47:37]

Can the tests properly initialize things? I ask as it would be nice if this
could return a const ref rather than a value.

[robliao, 2016/02/03 22:47:20]

Clarified the comment. 

The nature of search functions makes it tricky to hand out const refs since the
search can fail. The alternative here is to have a
base::LazyInstance<ScreenWinDisplay> for cases when this may happen and then we
can avoid all these copies (though in theory, the compiler should be optimizing
these copies out). If you'd prefer that, I could put that here.

The DCHECK is here to ensure 1:1 correspondence between MONITORINFOEX and
ScreenWinDisplay. However, if there are no displays, then we had out the default
display as a fallback.

The test comment doesn't apply anymore as we'll be checking for a ScreenWin
before making these calls.

+  DCHECK_EQ(screen_win_displays_.size(), 0u);
+  return gfx::win::ScreenWinDisplay();
 }
 
 }  // namespace gfx