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

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 ea74c8245f742ce70071d8520827b85a56d92830..c399571ada8ede70c98818dc133a0cd121e89529 100644
--- a/ui/gfx/screen_win.cc
+++ b/ui/gfx/screen_win.cc
@@ -1,4 +1,4 @@
-// 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.
 
@@ -6,8 +6,13 @@
 
 #include <windows.h>
 
+#include "base/logging.h"
+#include "ui/gfx/monitor.h"
+
 namespace {
 
+gfx::SimpleMonitor* monitor = NULL;

[Ben Goodger (Google), 2012/04/10 16:37:20]

g_monitor

[oshima, 2012/04/10 21:31:46]

Done.

+
 MONITORINFO GetMonitorInfoForMonitor(HMONITOR monitor) {
   MONITORINFO monitor_info = { 0 };
   monitor_info.cbSize = sizeof(monitor_info);
@@ -15,6 +20,24 @@ MONITORINFO GetMonitorInfoForMonitor(HMONITOR monitor) {
   return monitor_info;
 }
 
+gfx::Monitor* GetEmptyMonitor() {
+  if (!monitor) {
+    monitor = new gfx::SimpleMonitor();

[Ben Goodger (Google), 2012/04/10 16:37:20]

nit: no braces

[oshima, 2012/04/10 21:31:46]

Done.

+  }
+  monitor->set_bounds(gfx::Rect());

[Ben Goodger (Google), 2012/04/10 16:37:20]

since you're using a global, if anyone stores a copy of the gfx::Monitor* they
get from any of the Screen functions, and then someone else calls GetMonitor for
a different mi, the other place's monitor is going to change out from under
them. seems like you have to create a new monitor every time when you return it.

alternatively you could replace your virtual monitor interface with a details
struct like what windows uses (MONITORINFO) and just have the functions below
populate this struct on each platform.

[oshima, 2012/04/10 21:31:46]

 This is temporary until we have MonitorObserver implementation on other
platorms
(see my comment in screen.h)
 I'm working with Panels team to fix this now, as they need this feature too
to reposition panels when monitor changes. Notification is also somewhat broken
when monitor changes due to the lack of this feature on other platforms.
I filed a bug (crbug.com/122863) and put it to the comment in screen.h

+  monitor->set_work_area(gfx::Rect());
+  return monitor;
+}
+
+gfx::Monitor* GetMonitor(MONITORINFO& monitor_info) {
+  if (!monitor) {
+    monitor = new gfx::SimpleMonitor();

[Ben Goodger (Google), 2012/04/10 16:37:20]

nit: no braces

[oshima, 2012/04/10 21:31:46]

Done.

+  }
+  monitor->set_bounds(gfx::Rect(monitor_info.rcMonitor));
+  monitor->set_work_area(gfx::Rect(monitor_info.rcWork));
+  return monitor;
+}
+
 }  // namespace
 
 namespace gfx {
@@ -27,79 +50,52 @@ gfx::Point Screen::GetCursorScreenPoint() {
 }
 
 // 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);
+gfx::NativeWindow Screen::GetWindowAtCursorScreenPoint() {
+  POINT location;
+  return GetCursorPos(&location) ? WindowFromPoint(location) : NULL;
+}
+
+// static
+int Screen::GetNumMonitors() {
+  return GetSystemMetrics(SM_CMONITORS);
 }
 
 // static
-gfx::Rect Screen::GetMonitorAreaNearestWindow(gfx::NativeWindow window) {
+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 gfx::Rect(monitor_info.rcMonitor);
+  return GetMonitor(monitor_info);
 }
 
-static gfx::Rect GetMonitorAreaOrWorkAreaNearestPoint(const gfx::Point& point,
-                                                      bool work_area) {
+// 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 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);
+    return GetMonitor(mi);
+  return GetEmptyMonitor();
 }
 
 // static
-gfx::Rect Screen::GetPrimaryMonitorWorkArea() {
-  return gfx::Rect(GetMonitorInfoForMonitor(MonitorFromWindow(NULL,
-      MONITOR_DEFAULTTOPRIMARY)).rcWork);
+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
-gfx::Rect Screen::GetPrimaryMonitorBounds() {
-  return gfx::Rect(GetMonitorInfoForMonitor(MonitorFromWindow(NULL,
-      MONITOR_DEFAULTTOPRIMARY)).rcMonitor);
-}
-
-// static
-gfx::Rect Screen::GetMonitorWorkAreaMatching(const gfx::Rect& match_rect) {
+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 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);
+  return GetMonitor(monitor_info);
 }
 
 }  // namespace gfx