Reposition the Disconnect dialog when work area dimensions change.

remoting/host/disconnect_window_win.cc

Index: remoting/host/disconnect_window_win.cc
diff --git a/remoting/host/disconnect_window_win.cc b/remoting/host/disconnect_window_win.cc
index 06434a874a33e136540f2da44c8fdb8a11a3dac6..1ecc5deb0b3f08c11ce9d7d0bac9fb2fdf4545e0 100644
--- a/remoting/host/disconnect_window_win.cc
+++ b/remoting/host/disconnect_window_win.cc
@@ -51,6 +51,7 @@ private:
   void ShutdownHost();
   void EndDialog();
   void SetStrings(const UiStrings& strings, const std::string& username);
+  void SetDialogPosition();
 
   DisconnectCallback disconnect_callback_;
   HWND hwnd_;
@@ -109,6 +110,17 @@ BOOL DisconnectWindowWin::OnDialogMessage(HWND hwnd, UINT msg,
       hwnd_ = NULL;
       return TRUE;
 
+    // Ensure the dialog stays visible if the work area dimensions change.
+    case WM_SETTINGCHANGE:
+      if (wParam == SPI_SETWORKAREA)
+        SetDialogPosition();
+      return FALSE;

[alexeypa (please no reviews), 2012/08/08 22:05:08]

Return TRUE if the message was processed, FALSE otherwise. See
http://msdn.microsoft.com/en-us/library/windows/desktop/ms645469(v=vs.85).aspx.

[Wez, 2012/08/09 21:58:08]

We'd only do that if we wanted to suppress any additional processing that the
dialog message handler might do; in this case I don't think it makes any
difference, so TRUE is fine.

[Wez, 2012/08/09 21:58:08]

Done.

+
+    // Ensure the dialog stays visible if the display dimensions change.
+    case WM_DISPLAYCHANGE:
+      SetDialogPosition();
+      return FALSE;

[alexeypa (please no reviews), 2012/08/08 22:05:08]

return TRUE;

[Wez, 2012/08/09 21:58:08]

Same here.

[Wez, 2012/08/09 21:58:08]

Done.

+
     // Handle the disconnect hot-key.
     case WM_HOTKEY:
       EndDialog();
@@ -121,21 +133,20 @@ BOOL DisconnectWindowWin::OnDialogMessage(HWND hwnd, UINT msg,
       SetWindowLong(hwnd, DWL_MSGRESULT, HTCAPTION);
       return TRUE;
 
-    case WM_PAINT:
+    case WM_PAINT: {
+      PAINTSTRUCT ps;
+      HDC hdc = BeginPaint(hwnd_, &ps);

[alexeypa (please no reviews), 2012/08/08 22:05:08]

BeginPaint can return NULL.

[Wez, 2012/08/09 21:58:08]

Technically yes, but unless your system is horribly b0rked,it won't. All we can
do if it fails is not try to render; if we continue and try to render but don't
check the results of the API calls then the effect is the same...

[alexeypa (please no reviews), 2012/08/10 17:28:01]

The default assumption a person reading the code makes is that any error
returned from an API should be handled to make sure that the code does not rely
on uninitialized data. So either you need to explain each time why ignoring the
result is OK (just like it is done below for FindWindow() and
MonitorFromWindow()) or you need to check the error code.

In this specific example:
- GetClientRect can fail leaving garbage in |rect|, which later will passed back
to RoundRect().
- EndPaint can be called with uninitialized |ps|.

The rest of the errors should be OK, because NULLs are handled much better than
uninitialized data.

PS. Yes, Win32 API do pretty good job on validating the parameters but they are
not perfect.

[Wez, 2012/08/10 20:06:17]

I think this specific code should be pretty safe, but I'll create a bug to clean
this up; it doesn't belong in this CL.

[alexeypa (please no reviews), 2012/08/10 20:30:03]

OK, thanks!

+      RECT rect;
+      GetClientRect(hwnd_, &rect);

[alexeypa (please no reviews), 2012/08/08 22:05:08]

GetClientRect can fail.

       {
-        PAINTSTRUCT ps;
-        HDC hdc = BeginPaint(hwnd_, &ps);
-        RECT rect;
-        GetClientRect(hwnd_, &rect);
-        {
-          base::win::ScopedSelectObject border(hdc, border_pen_);
-          base::win::ScopedSelectObject brush(hdc, GetStockObject(NULL_BRUSH));
-          RoundRect(hdc, rect.left, rect.top, rect.right - 1, rect.bottom - 1,
-                    kWindowBorderRadius, kWindowBorderRadius);
-        }
-        EndPaint(hwnd_, &ps);
-        return TRUE;
+        base::win::ScopedSelectObject border(hdc, border_pen_);
+        base::win::ScopedSelectObject brush(hdc, GetStockObject(NULL_BRUSH));

[alexeypa (please no reviews), 2012/08/08 22:05:08]

GetStockObject can fail.

+        RoundRect(hdc, rect.left, rect.top, rect.right - 1, rect.bottom - 1,

[alexeypa (please no reviews), 2012/08/08 22:05:08]

RoundRect can fail.

+                  kWindowBorderRadius, kWindowBorderRadius);
       }
+      EndPaint(hwnd_, &ps);
+      return TRUE;

[alexeypa (please no reviews), 2012/08/08 22:05:08]

The application should return TRUE it processes WM_PAINT.

[Wez, 2012/08/09 21:58:08]

Yes, it is doing.

[alexeypa (please no reviews), 2012/08/10 17:28:01]

:-) I'm not sure what my comment was doing there.

+    }
   }
   return FALSE;
 }
@@ -182,22 +193,7 @@ void DisconnectWindowWin::Show(ChromotingHost* host,
   }
 
   SetStrings(host->ui_strings(), username);
-
-  // Try to center the window above the task-bar. If that fails, use the
-  // primary monitor. If that fails (very unlikely), use the default position.
-  HWND taskbar = FindWindow(L"Shell_TrayWnd", NULL);
-  HMONITOR monitor = MonitorFromWindow(taskbar, MONITOR_DEFAULTTOPRIMARY);
-  MONITORINFO monitor_info = {sizeof(monitor_info)};
-  RECT window_rect;
-  if (GetMonitorInfo(monitor, &monitor_info) &&
-      GetWindowRect(hwnd_, &window_rect)) {
-    int window_width = window_rect.right - window_rect.left;
-    int window_height = window_rect.bottom - window_rect.top;
-    int top = monitor_info.rcWork.bottom - window_height;
-    int left = (monitor_info.rcWork.right + monitor_info.rcWork.left -
-        window_width) / 2;
-    SetWindowPos(hwnd_, NULL, left, top, 0, 0, SWP_NOSIZE | SWP_NOZORDER);
-  }
+  SetDialogPosition();
   ShowWindow(hwnd_, SW_SHOW);
 }
 
@@ -273,6 +269,24 @@ void DisconnectWindowWin::SetStrings(const UiStrings& strings,
   SetWindowRgn(hwnd_, rgn, TRUE);
 }
 
+void DisconnectWindowWin::SetDialogPosition() {
+  // Try to center the window above the task-bar. If that fails, use the
+  // primary monitor. If that fails (very unlikely), use the default position.
+  HWND taskbar = FindWindow(L"Shell_TrayWnd", NULL);

[alexeypa (please no reviews), 2012/08/08 22:05:08]

nit: Define a constant for the window name.

[alexeypa (please no reviews), 2012/08/08 22:05:08]

All the API functions can fail.

[alexeypa (please no reviews), 2012/08/08 22:05:08]

It would be nice if the window could be correctly positioned even if the taskbar
is not present. Say we could use primary monitor boundaries if there is no
taskbar.

[Wez, 2012/08/09 21:58:08]

That's what this code does; see the comment and parameter to
MonitorFromWindow().

[Wez, 2012/08/09 21:58:08]

If FindWindow() fails then MonitorFromWindow() will fail and default to the
primary monitor. If MonitorFromWindow() fails then GetMonitorInfo() will fail
and we'll not try to reposition the window.

[Wez, 2012/08/09 21:58:08]

Done.

+  HMONITOR monitor = MonitorFromWindow(taskbar, MONITOR_DEFAULTTOPRIMARY);
+  MONITORINFO monitor_info = {sizeof(monitor_info)};
+  RECT window_rect;
+  if (GetMonitorInfo(monitor, &monitor_info) &&
+      GetWindowRect(hwnd_, &window_rect)) {
+    int window_width = window_rect.right - window_rect.left;
+    int window_height = window_rect.bottom - window_rect.top;
+    int top = monitor_info.rcWork.bottom - window_height;
+    int left = (monitor_info.rcWork.right + monitor_info.rcWork.left -
+        window_width) / 2;
+    SetWindowPos(hwnd_, NULL, left, top, 0, 0, SWP_NOSIZE | SWP_NOZORDER);
+  }
+}
+
 void DisconnectWindowWin::Hide() {
   EndDialog();
 }