Make the PIN confirmation dialog an ATL-based one and brushing it up a little bit.

Review URL: http://codereview.chromium.org/10266024

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134965 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 120 insertions, 100 deletions

diff --git a/remoting/host/elevated_controller.rc b/remoting/host/elevated_controller.rc
index 1d5cf83..d7cda15 100644
--- a/remoting/host/elevated_controller.rc
+++ b/remoting/host/elevated_controller.rc
@@ -21,23 +21,26 @@ BEGIN
 #endif
 END
 
+IDD_VERIFY_CONFIG_DIALOG    ICON    "remoting/resources/chromoting.ico"
 
 /////////////////////////////////////////////////////////////////////////////
 //
 // Dialog
 //
 
-IDD_VERIFY_CONFIG_DIALOG DIALOGEX 0, 0, 221, 120
+IDD_VERIFY_CONFIG_DIALOG DIALOGEX 0, 0, 221, 106
 STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | WS_POPUP | WS_CAPTION
 EXSTYLE WS_EX_TOPMOST
 CAPTION "kTitle"
 FONT 8, "MS Shell Dlg", 400, 0, 0x1
 BEGIN
-    EDITTEXT        IDC_PIN,7,69,107,15,WS_TABSTOP | ES_PASSWORD | ES_NUMBER
-    DEFPUSHBUTTON   "kOK",IDOK,116,100,98,14,WS_TABSTOP
     LTEXT           "kConfirmConfigChange",IDC_MESSAGE,7,7,207,30
-    LTEXT           "kEmail",IDC_EMAIL,7,38,207,30
-    PUSHBUTTON      "kCancel",IDCANCEL,7,100,98,14,WS_TABSTOP
+    LTEXT           "kAccount",IDC_EMAIL_LABEL,7,38,50,30
+    LTEXT           "kEmail",IDC_EMAIL,60,38,110,30
+    LTEXT           "kPin",IDC_PIN_LABEL,7,55,50,30
+    EDITTEXT        IDC_PIN,60,55,100,15,WS_TABSTOP | ES_PASSWORD | ES_NUMBER
+    DEFPUSHBUTTON   "kOK",IDOK,68,86,68,14,WS_TABSTOP
+    PUSHBUTTON      "kCancel",IDCANCEL,146,86,68,14,WS_TABSTOP
 END
 
 
diff --git a/remoting/host/elevated_controller_resource.h b/remoting/host/elevated_controller_resource.h
index 0376ed2..e92da46 100644
--- a/remoting/host/elevated_controller_resource.h
+++ b/remoting/host/elevated_controller_resource.h
@@ -10,5 +10,7 @@
 #define IDC_MESSAGE 102
 #define IDC_EMAIL 103
 #define IDC_PIN 104
+#define IDC_EMAIL_LABEL 105
+#define IDC_PIN_LABEL 106
 
 #endif  // REMOTING_HOST_ELEVATED_CONTROLLER_RESOURCE_H_
diff --git a/remoting/host/elevated_controller_win.cc b/remoting/host/elevated_controller_win.cc
index 4e7c9f5..9f2154a 100644
--- a/remoting/host/elevated_controller_win.cc
+++ b/remoting/host/elevated_controller_win.cc
@@ -195,7 +195,7 @@ HRESULT WriteConfig(const char* content, size_t length) {
 
   // Ask the user to verify the configuration.
   remoting::VerifyConfigWindowWin verify_win(email, host_id, host_secret_hash);
-  if (!verify_win.Run()) {
+  if (verify_win.DoModal() != IDOK) {
     return E_FAIL;
   }
 
diff --git a/remoting/host/verify_config_window_win.cc b/remoting/host/verify_config_window_win.cc
index 39c6a4a2..5f5da28 100644
--- a/remoting/host/verify_config_window_win.cc
+++ b/remoting/host/verify_config_window_win.cc
@@ -5,6 +5,7 @@
 #include "remoting/host/verify_config_window_win.h"
 
 #include <atlbase.h>
+#include <atlwin.h>
 #include <windows.h>
 
 #include "base/base64.h"
@@ -19,107 +20,114 @@ namespace remoting {
 
 VerifyConfigWindowWin::VerifyConfigWindowWin(const std::string& email,
     const std::string& host_id, const std::string& host_secret_hash)
-  : hwnd_(NULL),
-    email_(email),
+  : email_(email),
     host_id_(host_id),
     host_secret_hash_(host_secret_hash) {
 }
 
-VerifyConfigWindowWin::~VerifyConfigWindowWin() {
-  EndDialog();
+void VerifyConfigWindowWin::OnCancel(UINT code, int id, HWND control) {
+  EndDialog(IDCANCEL);
 }
 
-bool VerifyConfigWindowWin::Run() {
-  // TODO(simonmorris): Provide a handle of a parent window for this dialog.
-  return (DialogBoxParam(ATL::_AtlBaseModule.GetModuleInstance(),
-                         MAKEINTRESOURCE(IDD_VERIFY_CONFIG_DIALOG),
-                         NULL,
-                         (DLGPROC)DialogProc,
-                         (LPARAM)this) != 0);
+void VerifyConfigWindowWin::OnClose() {
+  EndDialog(IDCANCEL);
 }
 
-BOOL CALLBACK VerifyConfigWindowWin::DialogProc(HWND hwnd, UINT msg,
-                                                WPARAM wParam, LPARAM lParam) {
-  VerifyConfigWindowWin* win = NULL;
-  if (msg == WM_INITDIALOG) {
-    win = reinterpret_cast<VerifyConfigWindowWin*>(lParam);
-    CHECK(win);
-    SetWindowLongPtr(hwnd, DWLP_USER, (LONG_PTR)win);
-  } else {
-    LONG_PTR lp = GetWindowLongPtr(hwnd, DWLP_USER);
-    win = reinterpret_cast<VerifyConfigWindowWin*>(lp);
-  }
-  if (win == NULL)
-    return FALSE;
-  return win->OnDialogMessage(hwnd, msg, wParam, lParam);
+LRESULT VerifyConfigWindowWin::OnInitDialog(HWND wparam, LPARAM lparam) {
+  CenterWindow();
+
+  // TODO(simonmorris): l10n.
+  SetWindowText(L"Chrome Remote Desktop");
+
+  CWindow email_label(GetDlgItem(IDC_EMAIL_LABEL));
+  email_label.SetWindowText(L"Account:");
+
+  CWindow pin_label(GetDlgItem(IDC_PIN_LABEL));
+  pin_label.SetWindowText(L"PIN:");
+
+  CWindow ok_button(GetDlgItem(IDOK));
+  ok_button.SetWindowText(L"Confirm");
+
+  CWindow cancel_button(GetDlgItem(IDCANCEL));
+  cancel_button.SetWindowText(L"Cancel");
+
+  CWindow message_text(GetDlgItem(IDC_MESSAGE));
+  message_text.SetWindowText(L"Please confirm your account and PIN below to "
+                             L"allow access by Chrome Remote Desktop.");
+
+  CWindow email_text(GetDlgItem(IDC_EMAIL));
+  email_text.SetWindowText(UTF8ToUTF16(email_).c_str());
+  return TRUE;
 }
 
-BOOL VerifyConfigWindowWin::OnDialogMessage(HWND hwnd, UINT msg,
-                                            WPARAM wParam, LPARAM lParam) {
-  switch (msg) {
-    case WM_INITDIALOG:
-      hwnd_ = hwnd;
-      InitDialog();
-      return TRUE;
-    case WM_DESTROY:
-      ::EndDialog(hwnd, 0);
-    case WM_COMMAND:
-      switch (LOWORD(wParam)) {
-        case IDOK:
-          ::EndDialog(hwnd, VerifyHostSecretHash());
-          hwnd_ = NULL;
-          return TRUE;
-        case IDCANCEL:
-          ::EndDialog(hwnd, 0);
-          hwnd_ = NULL;
-          return TRUE;
-      }
+void VerifyConfigWindowWin::OnOk(UINT code, int id, HWND control) {
+  if (VerifyHostSecretHash()) {
+    EndDialog(IDOK);
+  } else {
+    EndDialog(IDCANCEL);
   }
-  return FALSE;
 }
 
-void VerifyConfigWindowWin::InitDialog() {
-  // TODO(simonmorris): l10n.
-  SetWindowText(hwnd_, L"Chrome Remote Desktop");
+void VerifyConfigWindowWin::CenterWindow() {
+  // Get the window dimensions.
+  RECT rect;
+  if (!GetWindowRect(&rect)) {
+    return;
+  }
 
-  HWND hwndOk = GetDlgItem(hwnd_, IDOK);
-  CHECK(hwndOk);
-  SetWindowText(hwndOk, L"OK");
+  // Center against the owner window unless it is minimized or invisible.
+  HWND owner = ::GetWindow(m_hWnd, GW_OWNER);
+  if (owner != NULL) {
+    DWORD style = ::GetWindowLong(owner, GWL_STYLE);
+    if ((style & WS_MINIMIZE) != 0 || (style & WS_VISIBLE) == 0) {
+      owner = NULL;
+    }
+  }
 
-  HWND hwndCancel = GetDlgItem(hwnd_, IDCANCEL);
-  CHECK(hwndCancel);
-  SetWindowText(hwndCancel, L"Cancel");
+  // Make sure that the window will not end up split by a monitor's boundary.
+  RECT area_rect;
+  if (!::SystemParametersInfo(SPI_GETWORKAREA, NULL, &area_rect, NULL)) {
+    return;
+  }
 
-  HWND hwndMessage = GetDlgItem(hwnd_, IDC_MESSAGE);
-  CHECK(hwndMessage);
-  SetWindowText(hwndMessage, L"To confirm that your Chrome Remote Desktop "
-      L"should be accessible by this account, please enter your PIN below.");
+  // On a multi-monitor system use the monitor where the owner window is shown.
+  RECT owner_rect = area_rect;
+  if (owner != NULL && ::GetWindowRect(owner, &owner_rect)) {
+    HMONITOR monitor = ::MonitorFromRect(&owner_rect, MONITOR_DEFAULTTONEAREST);
+    if (monitor != NULL) {
+      MONITORINFO monitor_info = {0};
+      monitor_info.cbSize = sizeof(monitor_info);
+      if (::GetMonitorInfo(monitor, &monitor_info)) {
+        area_rect = monitor_info.rcWork;
+      }
+    }
+  }
 
-  HWND hwndEmail = GetDlgItem(hwnd_, IDC_EMAIL);
-  CHECK(hwndEmail);
-  SetWindowText(hwndEmail, UTF8ToUTF16(email_).c_str());
+  LONG width = rect.right - rect.left;
+  LONG height = rect.bottom - rect.top;
+  LONG x  = (owner_rect.left + owner_rect.right - width) / 2;
+  LONG y = (owner_rect.top + owner_rect.bottom - height) / 2;
 
-  HWND hwndPin = GetDlgItem(hwnd_, IDC_PIN);
-  CHECK(hwndPin);
-  SetFocus(hwndPin);
-}
+  x = std::max(x, area_rect.left);
+  x = std::min(x, area_rect.right - width);
+  y = std::max(y, area_rect.top);
+  y = std::min(y, area_rect.bottom - width);
 
-void VerifyConfigWindowWin::EndDialog() {
-  if (hwnd_) {
-    ::EndDialog(hwnd_, 0);
-    hwnd_ = NULL;
-  }
+  SetWindowPos(NULL, x, y, -1, -1, SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE);
 }
 
 bool VerifyConfigWindowWin::VerifyHostSecretHash() {
-  const int kMaxPinLength = 256;
-  // TODO(simonmorris): Use ATL's string class, if it's more convenient.
-  scoped_array<WCHAR> pinWSTR(new WCHAR[kMaxPinLength]);
-  HWND hwndPin = GetDlgItem(hwnd_, IDC_PIN);
-  CHECK(hwndPin);
-  GetWindowText(hwndPin, pinWSTR.get(), kMaxPinLength);
-  std::string pin(UTF16ToUTF8(pinWSTR.get()));
-  return VerifyHostPinHash(host_secret_hash_, host_id_, pin);
+  CWindow pin_edit(GetDlgItem(IDC_PIN));
+
+  // Get the PIN length.
+  int pin_length = pin_edit.GetWindowTextLength();
+  scoped_array<char16> pin(new char16[pin_length + 1]);
+
+  // Get the PIN making sure it is NULL terminated even if an error occurs.
+  int result = pin_edit.GetWindowText(pin.get(), pin_length + 1);
+  pin[std::min(result, pin_length)] = 0;
+
+  return VerifyHostPinHash(host_secret_hash_, host_id_, UTF16ToUTF8(pin.get()));
 }
 
 }  // namespace remoting
diff --git a/remoting/host/verify_config_window_win.h b/remoting/host/verify_config_window_win.h
index fc84427..b9709ed 100644
--- a/remoting/host/verify_config_window_win.h
+++ b/remoting/host/verify_config_window_win.h
@@ -5,34 +5,41 @@
 #ifndef REMOTING_HOST_VERIFY_CONFIG_WINDOW_WIN_H
 #define REMOTING_HOST_VERIFY_CONFIG_WINDOW_WIN_H
 
+#include <atlbase.h>
+#include <atlcrack.h>
+#include <atlwin.h>
 #include <string>
 
 #include "base/callback.h"
+#include "remoting/host/elevated_controller_resource.h"
 
 namespace remoting {
 
-// TODO(simonmorris): Derive this class from ATL's CDialog.
-class VerifyConfigWindowWin {
+class VerifyConfigWindowWin : public ATL::CDialogImpl<VerifyConfigWindowWin> {
  public:
-   VerifyConfigWindowWin(const std::string& email,
-                         const std::string& host_id,
-                         const std::string& host_secret_hash);
-  ~VerifyConfigWindowWin();
+  enum { IDD = IDD_VERIFY_CONFIG_DIALOG };
 
-  // Run the dialog modally. Returns true on successful verification.
-  bool Run();
+  BEGIN_MSG_MAP_EX(VerifyConfigWindowWin)
+    MSG_WM_INITDIALOG(OnInitDialog)
+    MSG_WM_CLOSE(OnClose)
+    COMMAND_ID_HANDLER_EX(IDOK, OnOk)
+    COMMAND_ID_HANDLER_EX(IDCANCEL, OnCancel)
+  END_MSG_MAP()
 
- private:
-  static BOOL CALLBACK DialogProc(HWND hwmd, UINT msg, WPARAM wParam,
-                                  LPARAM lParam);
+  VerifyConfigWindowWin(const std::string& email,
+                        const std::string& host_id,
+                        const std::string& host_secret_hash);
 
-  BOOL OnDialogMessage(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);
+  void OnCancel(UINT code, int id, HWND control);
+  void OnClose();
+  LRESULT OnInitDialog(HWND wparam, LPARAM lparam);
+  void OnOk(UINT code, int id, HWND control);
 
-  void InitDialog();
-  void EndDialog();
+ private:
+  // Centers the dialog window against the owner window.
+  void CenterWindow();
   bool VerifyHostSecretHash();
 
-  HWND hwnd_;
   const std::string email_;
   const std::string host_id_;
   const std::string host_secret_hash_;