Reposition the Disconnect dialog when work area dimensions change.

remoting/host/disconnect_window_win.cc

 // 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 "remoting/host/disconnect_window.h"
 
 #include <windows.h>
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
@@ skipping 33 matching lines @@
 
 private:
   static BOOL CALLBACK DialogProc(HWND hwmd, UINT msg, WPARAM wParam,
                                   LPARAM lParam);
 
   BOOL OnDialogMessage(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);
 
   void ShutdownHost();
   void EndDialog();
   void SetStrings(const UiStrings& strings, const std::string& username);
+  void SetDialogPosition();
 
   DisconnectCallback disconnect_callback_;
   HWND hwnd_;
   bool has_hotkey_;
   base::win::ScopedGDIObject<HPEN> border_pen_;
 
   DISALLOW_COPY_AND_ASSIGN(DisconnectWindowWin);
 };
 
 DisconnectWindowWin::DisconnectWindowWin()
@@ skipping 38 matching lines @@
           ShutdownHost();
           return TRUE;
       }
       return FALSE;
 
     // Ensure we don't try to use the HWND anymore.
     case WM_DESTROY:
       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();
       ShutdownHost();
       return TRUE;
 
     // Let the window be draggable by its client area by responding
     // that the entire window is the title bar.
     case WM_NCHITTEST:
       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;
 }
 
 void DisconnectWindowWin::Show(ChromotingHost* host,
                                const DisconnectCallback& disconnect_callback,
                                const std::string& username) {
   CHECK(!hwnd_);
   disconnect_callback_ = disconnect_callback;
 
@@ skipping 26 matching lines @@
                                     (DLGPROC)DialogProc, (LPARAM)this);
   CHECK(hwnd_);
 
   // Set up handler for Ctrl-Alt-Esc shortcut.
   if (!has_hotkey_ && RegisterHotKey(hwnd_, DISCONNECT_HOTKEY_ID,
                                      MOD_ALT | MOD_CONTROL, VK_ESCAPE)) {
     has_hotkey_ = true;
   }
 
   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);
 }
 
 void DisconnectWindowWin::ShutdownHost() {
   CHECK(!disconnect_callback_.is_null());
   disconnect_callback_.Run();
 }
 
 static int GetControlTextWidth(HWND control) {
   RECT rect = {0, 0, 0, 0};
@@ skipping 55 matching lines @@
   GetWindowRect(hwnd_, &window_rect);
   int width = (window_rect.right - window_rect.left) +
       button_width_delta + label_width_delta;
   int height = window_rect.bottom - window_rect.top;
   SetWindowPos(hwnd_, NULL, 0, 0, width, height, SWP_NOMOVE|SWP_NOZORDER);
   HRGN rgn = CreateRoundRectRgn(0, 0, width, height, kWindowBorderRadius,
                                 kWindowBorderRadius);
   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();
 }
 
 void DisconnectWindowWin::EndDialog() {
   if (has_hotkey_) {
     UnregisterHotKey(hwnd_, DISCONNECT_HOTKEY_ID);
     has_hotkey_ = false;
   }
 
   if (hwnd_) {
     ::DestroyWindow(hwnd_);
     hwnd_ = NULL;
   }
 }
 
 scoped_ptr<DisconnectWindow> DisconnectWindow::Create() {
   return scoped_ptr<DisconnectWindow>(new DisconnectWindowWin());
 }
 
 }  // namespace remoting