Add support for Disconnect hot key in Windows.

remoting/host/disconnect_window_win.cc

Index: remoting/host/disconnect_window_win.cc
===================================================================
--- remoting/host/disconnect_window_win.cc	(revision 96410)
+++ remoting/host/disconnect_window_win.cc	(working copy)
@@ -25,6 +25,8 @@
 //   SimpleHost: simple_host_process.cc
 extern HMODULE g_hModule;
 
+const int DISCONNECT_HOTKEY_ID = 1000;

[Wez, 2011/08/11 22:15:24]

Is there any significance to this choice of value?  If not then could we get rid
of this ID and just pass 0 to the API for the ID, given that we don't check the
ID in any case?

[garykac, 2011/08/11 23:04:49]

It must be between 0x0000 and 0xbfff.

Having a named constant seems far preferable (from a code-readability POV) to
passing a mysterious a '0' value to a function. Esp. since it needs to be
consistent for multiple calls.

+
 namespace remoting {
 
 class DisconnectWindowWin : public DisconnectWindow {
@@ -42,11 +44,14 @@
 
   BOOL OnDialogMessage(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);
 
+  void ShutdownHost();
   void EndDialog();
+  void RemoveKeyHandler();
 
   remoting::ChromotingHost* host_;
   std::string username_;
   HWND hwnd_;
+  bool has_hotkey_;
 
   DISALLOW_COPY_AND_ASSIGN(DisconnectWindowWin);
 };
@@ -54,7 +59,8 @@
 DisconnectWindowWin::DisconnectWindowWin()
     : host_(NULL),
       username_(""),
-      hwnd_(NULL) {
+      hwnd_(NULL),
+      has_hotkey_(false) {
 }
 
 DisconnectWindowWin::~DisconnectWindowWin() {
@@ -89,7 +95,8 @@
         HWND hwndButton = GetDlgItem(hwnd, IDC_DISCONNECT);
         CHECK(hwndButton);
         std::wstring w_button = UTF8ToWide(kDisconnectButton);
-        w_button += UTF8ToWide(kDisconnectKeysWin);
+        if (has_hotkey_)
+          w_button += UTF8ToWide(kDisconnectKeysWin);
         SetWindowText(hwndButton, w_button.c_str());
 
         HWND hwndSharingWith = GetDlgItem(hwnd, IDC_DISCONNECT_SHARINGWITH);
@@ -104,6 +111,12 @@
         SetWindowText(hwndUsername, w_username.c_str());
       }
       return TRUE;
+    case WM_HOTKEY:
+      {
+        ShutdownHost();
+        EndDialog();
+      }
+      return TRUE;
     case WM_CLOSE:
       // Ignore close messages.
       return TRUE;
@@ -115,8 +128,8 @@
       switch (LOWORD(wParam)) {
         case IDC_DISCONNECT:
           {
-            CHECK(host_);
-            host_->Shutdown(NULL);
+            ShutdownHost();
+            RemoveKeyHandler();
             ::EndDialog(hwnd, LOWORD(wParam));
             hwnd_ = NULL;

[Wez, 2011/08/11 22:15:24]

These lines look almost identical to EndDialog() below; why not just allow it to
take a result parameter so it can be used here?

[garykac, 2011/08/11 23:04:49]

I was sortof trying to avoid having a |result| param in EndDialog since it's 0
everywhere except here.

But it's better to share code, so I changed it.

[garykac, 2011/08/11 23:04:49]

I was sortof trying to avoid having a |result| param in EndDialog since it's 0
everywhere except here. But I was bothered by the multiple |EndDialog|s.

It's better to share code, so I changed it.

           }
@@ -140,19 +153,37 @@
   }
 
   ShowWindow(hwnd_, SW_SHOW);
+
+  // Set up handler for Ctrl-Alt-Esc shortcut.
+  if (!has_hotkey_) {
+    RegisterHotKey(hwnd_, DISCONNECT_HOTKEY_ID, MOD_ALT | MOD_CONTROL,
+                   VK_ESCAPE);

[Wez, 2011/08/11 22:15:24]

I thought we were going to not display the hot-key test unless we actually
successfully registered the hot-key?

[garykac, 2011/08/11 23:04:49]

Added SetDisconnectButtonText call here. Previously, this was relying on
WM_INITDIALOG to do it (which doesn't work).

[Wez, 2011/08/11 23:58:30]

My point is not about when you set the button text, but the fact that since
you're not checking the result of RegisterHotKey, you'll always display the
hot-key text on the button, even if the hot-key wasn't successfully registered.

[garykac, 2011/08/12 00:23:16]

Argh. I completely missed that point (which Jamie made as well). Done.

+    has_hotkey_ = true;
+  }
 }
 
+void DisconnectWindowWin::ShutdownHost() {
+  CHECK(host_);
+  host_->Shutdown(NULL);
+}
+
 void DisconnectWindowWin::Hide() {
   EndDialog();
 }
 
 void DisconnectWindowWin::EndDialog() {
+  RemoveKeyHandler();
   if (hwnd_) {
     ::EndDialog(hwnd_, 0);
     hwnd_ = NULL;
   }
 }
 
+void DisconnectWindowWin::RemoveKeyHandler() {

[Wez, 2011/08/11 22:15:24]

nit: Does this really need to be a separate function?

[garykac, 2011/08/11 23:04:49]

*need*? No.  But it's called from multiple places and is convenient to have one
location that performs all the unregister tasks.

[Wez, 2011/08/11 23:58:30]

It's called from two places, both of which then do EndDialog() things, don't
they?

[garykac, 2011/08/12 00:23:16]

Actually it's called in 1 place now that EndDialog has been merged.  So it's
gone.

+  UnregisterHotKey(hwnd_, DISCONNECT_HOTKEY_ID);
+  has_hotkey_ = false;
+}
+
 DisconnectWindow* DisconnectWindow::Create() {
   return new DisconnectWindowWin;
 }