Refactor screen/monitor so that gfx::Screen returns monitor object.

ui/gfx/screen_win.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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 "ui/gfx/screen.h"
 
 #include <windows.h>
 
+#include "base/logging.h"
+#include "ui/gfx/monitor.h"
+
 namespace {
 
+gfx::SimpleMonitor* g_monitor = NULL;
+
 MONITORINFO GetMonitorInfoForMonitor(HMONITOR monitor) {
   MONITORINFO monitor_info = { 0 };
   monitor_info.cbSize = sizeof(monitor_info);
   GetMonitorInfo(monitor, &monitor_info);
   return monitor_info;
 }
 
+gfx::Monitor* GetEmptyMonitor() {

[Ben Goodger (Google), 2012/04/11 21:05:58]

So even though you say it's temporary, it really troubles me that we'd add code
that has this bug. It means that code even within the same call stack could have
unpredictable behavior if any of these functions are called with different
monitors. I can't guarantee that doesn't happen in Chrome today, so as a result
I'm not really comfortable with this going in even temporarily.

Here's my question.

Monitor is actually pretty simple. It's bounds, work area dimensions, scale
factor etc. i.e. it's just data. It should just be a struct, like MONITORINFO,
but cross platform. The gfx::Screen methods that currently return a
gfx::Monitor* could take a gfx::Monitor* out param and prefill the struct the
caller holds on the stack.

If you need this struct to carry a handle to the native monitor you can invent a
typedef for that too.

[oshima, 2012/04/11 22:05:07]

Ok, that's good suggestion. I'll make that change.

+  if (!g_monitor)
+    g_monitor = new gfx::SimpleMonitor();
+  g_monitor->set_bounds(gfx::Rect());
+  g_monitor->set_work_area(gfx::Rect());
+  return g_monitor;
+}
+
+gfx::Monitor* GetMonitor(MONITORINFO& monitor_info) {
+  if (!g_monitor)
+    g_monitor = new gfx::SimpleMonitor();
+  g_monitor->set_bounds(gfx::Rect(monitor_info.rcMonitor));
+  g_monitor->set_work_area(gfx::Rect(monitor_info.rcWork));
+  return g_monitor;
+}
+
 }  // namespace
 
 namespace gfx {
 
 // static
 gfx::Point Screen::GetCursorScreenPoint() {
   POINT pt;
   GetCursorPos(&pt);
   return gfx::Point(pt);
 }
 
 // static
-gfx::Rect Screen::GetMonitorWorkAreaNearestWindow(gfx::NativeWindow window) {
-  MONITORINFO monitor_info;
-  monitor_info.cbSize = sizeof(monitor_info);
-  GetMonitorInfo(MonitorFromWindow(window, MONITOR_DEFAULTTONEAREST),
-                 &monitor_info);
-  return gfx::Rect(monitor_info.rcWork);
-}
-
-// static
-gfx::Rect Screen::GetMonitorAreaNearestWindow(gfx::NativeWindow window) {
-  MONITORINFO monitor_info;
-  monitor_info.cbSize = sizeof(monitor_info);
-  GetMonitorInfo(MonitorFromWindow(window, MONITOR_DEFAULTTONEAREST),
-                 &monitor_info);
-  return gfx::Rect(monitor_info.rcMonitor);
-}
-
-static gfx::Rect GetMonitorAreaOrWorkAreaNearestPoint(const gfx::Point& point,
-                                                      bool work_area) {
-  POINT initial_loc = { point.x(), point.y() };
-  HMONITOR monitor = MonitorFromPoint(initial_loc, MONITOR_DEFAULTTONEAREST);
-  MONITORINFO mi = {0};
-  mi.cbSize = sizeof(mi);
-  if (monitor && GetMonitorInfo(monitor, &mi))
-    return gfx::Rect(work_area ? mi.rcWork : mi.rcMonitor);
-  return gfx::Rect();
-}
-
-// static
-gfx::Rect Screen::GetMonitorWorkAreaNearestPoint(const gfx::Point& point) {
-  return GetMonitorAreaOrWorkAreaNearestPoint(point, true);
-}
-
-// static
-gfx::Rect Screen::GetMonitorAreaNearestPoint(const gfx::Point& point) {
-  return GetMonitorAreaOrWorkAreaNearestPoint(point, false);
-}
-
-// static
-gfx::Rect Screen::GetPrimaryMonitorWorkArea() {
-  return gfx::Rect(GetMonitorInfoForMonitor(MonitorFromWindow(NULL,
-      MONITOR_DEFAULTTOPRIMARY)).rcWork);
-}
-
-// static
-gfx::Rect Screen::GetPrimaryMonitorBounds() {
-  return gfx::Rect(GetMonitorInfoForMonitor(MonitorFromWindow(NULL,
-      MONITOR_DEFAULTTOPRIMARY)).rcMonitor);
-}
-
-// static
-gfx::Rect Screen::GetMonitorWorkAreaMatching(const gfx::Rect& match_rect) {
-  RECT other_bounds_rect = match_rect.ToRECT();
-  MONITORINFO monitor_info = GetMonitorInfoForMonitor(MonitorFromRect(
-      &other_bounds_rect, MONITOR_DEFAULTTONEAREST));
-  return gfx::Rect(monitor_info.rcWork);
-}
-
-// static
 gfx::NativeWindow Screen::GetWindowAtCursorScreenPoint() {
   POINT location;
   return GetCursorPos(&location) ? WindowFromPoint(location) : NULL;
 }
 
 // static
-gfx::Size Screen::GetPrimaryMonitorSize() {
-  return gfx::Size(GetSystemMetrics(SM_CXSCREEN),
-                   GetSystemMetrics(SM_CYSCREEN));
-}
-
-// static
 int Screen::GetNumMonitors() {
   return GetSystemMetrics(SM_CMONITORS);
 }
 
+// static
+const gfx::Monitor* Screen::GetMonitorNearestWindow(gfx::NativeWindow window) {
+  MONITORINFO monitor_info;
+  monitor_info.cbSize = sizeof(monitor_info);
+  GetMonitorInfo(MonitorFromWindow(window, MONITOR_DEFAULTTONEAREST),
+                 &monitor_info);
+  return GetMonitor(monitor_info);
+}
+
+// static
+const gfx::Monitor* Screen::GetMonitorNearestPoint(const gfx::Point& point) {
+  POINT initial_loc = { point.x(), point.y() };
+  HMONITOR monitor = MonitorFromPoint(initial_loc, MONITOR_DEFAULTTONEAREST);
+  MONITORINFO mi = {0};
+  mi.cbSize = sizeof(mi);
+  if (monitor && GetMonitorInfo(monitor, &mi))
+    return GetMonitor(mi);
+  return GetEmptyMonitor();
+}
+
+// static
+const gfx::Monitor* Screen::GetPrimaryMonitor() {
+  MONITORINFO mi = GetMonitorInfoForMonitor(
+      MonitorFromWindow(NULL, MONITOR_DEFAULTTOPRIMARY));
+  gfx::Monitor* monitor = GetMonitor(mi);
+  DCHECK_EQ(GetSystemMetrics(SM_CXSCREEN), monitor->GetSize().width());
+  DCHECK_EQ(GetSystemMetrics(SM_CYSCREEN), monitor->GetSize().height());
+  return monitor;
+}
+
+// static
+const gfx::Monitor* Screen::GetMonitorMatching(const gfx::Rect& match_rect) {
+  RECT other_bounds_rect = match_rect.ToRECT();
+  MONITORINFO monitor_info = GetMonitorInfoForMonitor(MonitorFromRect(
+      &other_bounds_rect, MONITOR_DEFAULTTONEAREST));
+  return GetMonitor(monitor_info);
+}
+
 }  // namespace gfx