Adding retry logic for applying desktop resolution changes to RDP session.

remoting/host/win/rdp_client_window.cc

 // Copyright (c) 2013 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/win/rdp_client_window.h"
 
 #include <wtsdefs.h>
 
 #include <list>
 
+#include "base/bind.h"
 #include "base/lazy_instance.h"
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_local.h"
 #include "base/win/scoped_bstr.h"
 
 namespace remoting {
 
 namespace {
 
 // RDP session disconnect reason codes that should not be interpreted as errors.
 constexpr long kDisconnectReasonNoInfo = 0;
 constexpr long kDisconnectReasonLocalNotError = 1;
 constexpr long kDisconnectReasonRemoteByUser = 2;
 constexpr long kDisconnectReasonByServer = 3;
 
 // Maximum length of a window class name including the terminating nullptr.
 constexpr int kMaxWindowClassLength = 256;
 
 // Each member of the array returned by GetKeyboardState() contains status data
 // for a virtual key. If the high-order bit is 1, the key is down; otherwise, it
 // is up.
 constexpr BYTE kKeyPressedFlag = 0x80;
 
 constexpr int kKeyboardStateLength = 256;
 
+constexpr base::TimeDelta kReapplyResolutionPeriod =
+    base::TimeDelta::FromMilliseconds(250);
+
+// We want to try to reapply resolution changes for ~5 seconds (20 * 250ms).
+constexpr int kMaxResolutionReapplyAttempts = 20;
+
 // The RDP control creates 'IHWindowClass' window to handle keyboard input.
 constexpr wchar_t kRdpInputWindowClass[] = L"IHWindowClass";
 
 enum RdpAudioMode {
   // Redirect sounds to the client. This is the default value.
   kRdpAudioModeRedirect = 0,
 
   // Play sounds at the remote computer. Equivalent to |kRdpAudioModeNone| if
   // the remote computer is running a server SKU.
   kRdpAudioModePlayOnServer = 1,
@@ skipping 159 matching lines @@
   SendMessage(input_window, WM_KEYUP, VK_MENU, MAKELPARAM(1, alt | up));
 
   // Release 'Ctrl'.
   keyboard_state[VK_CONTROL] &= ~kKeyPressedFlag;
   keyboard_state[VK_LCONTROL] &= ~kKeyPressedFlag;
   CHECK(SetKeyboardState(keyboard_state));
   SendMessage(input_window, WM_KEYUP, VK_CONTROL, MAKELPARAM(1, control | up));
 }
 
 void RdpClientWindow::ChangeResolution(const ScreenResolution& resolution) {
+  // Stop any pending resolution changes.
+  apply_resolution_timer_.Stop();
   screen_resolution_ = resolution;
-  UpdateDesktopResolution();
+  HRESULT result = UpdateDesktopResolution();
+  if (FAILED(result)) {
+    LOG(WARNING) << "UpdateSessionDisplaySettings() failed: 0x" << std::hex
+                 << result;
+  }
 }
 
 void RdpClientWindow::OnClose() {
   if (!client_.get()) {
     NotifyDisconnected();
     return;
   }
 
   // Request a graceful shutdown.
   mstsc::ControlCloseStatus close_status;
   HRESULT result = client_->RequestClose(&close_status);
   if (FAILED(result)) {
     LOG(ERROR) << "Failed to request a graceful shutdown of an RDP connection"
-               << ", result=0x" << std::hex << result << std::dec;
+               << ", result=0x" << std::hex << result;
     NotifyDisconnected();
     return;
   }
 
   if (close_status != mstsc::controlCloseWaitForEvents) {
     NotifyDisconnected();
     return;
   }
 
   // Expect IMsTscAxEvents::OnConfirmClose() or IMsTscAxEvents::OnDisconnect()
@@ skipping 139 matching lines @@
   if (FAILED(result))
     return LogOnCreateError(result);
 
   return 0;
 }
 
 void RdpClientWindow::OnDestroy() {
   client_.Release();
   client_9_.Release();
   client_settings_.Release();
+  apply_resolution_timer_.Stop();
 }
 
 HRESULT RdpClientWindow::OnAuthenticationWarningDisplayed() {
   LOG(WARNING) << "RDP: authentication warning is about to be shown.";
 
   // Hook window activation to cancel any modal UI shown by the RDP control.
   // This does not affect creation of other instances of the RDP control on this
   // thread because the RDP control's window is hidden and is not activated.
   window_activate_hook_ = WindowHook::Create();
   return S_OK;
@@ skipping 11 matching lines @@
 
   NotifyConnected();
   return S_OK;
 }
 
 HRESULT RdpClientWindow::OnLoginComplete() {
   VLOG(1) << "RDP: user successfully logged in.";
 
   user_logged_in_ = true;
 
-  // Apply pending screen size changes to the desktop.
-  UpdateDesktopResolution();
+  // Set up a timer to periodically apply pending screen size changes to the
+  // desktop.  Attempting to set the resolution now seems to fail consistently,
+  // but succeeds after a brief timeout.
+  if (client_9_) {
+    apply_resolution_attempts_ = 0;
+    apply_resolution_timer_.Start(
+        FROM_HERE, kReapplyResolutionPeriod,
+        base::Bind(&RdpClientWindow::ReapplyDesktopResolution, this));
+  }
 
   return S_OK;
 }
 
 HRESULT RdpClientWindow::OnDisconnected(long reason) {
   if (reason == kDisconnectReasonNoInfo ||
       reason == kDisconnectReasonLocalNotError ||
       reason == kDisconnectReasonRemoteByUser ||
       reason == kDisconnectReasonByServer) {
     VLOG(1) << "RDP: disconnected from " << server_endpoint_.ToString()
@@ skipping 37 matching lines @@
 
 HRESULT RdpClientWindow::OnConfirmClose(VARIANT_BOOL* allow_close) {
   *allow_close = VARIANT_TRUE;
 
   NotifyDisconnected();
   return S_OK;
 }
 
 int RdpClientWindow::LogOnCreateError(HRESULT error) {
   LOG(ERROR) << "RDP: failed to initiate a connection to "
-             << server_endpoint_.ToString() << ": error="
-             << std::hex << error << std::dec;
+             << server_endpoint_.ToString() << ": error=" << std::hex << error;
   client_.Release();
   client_9_.Release();
   client_settings_.Release();
   return -1;
 }
 
 void RdpClientWindow::NotifyConnected() {
   if (event_handler_) {
     event_handler_->OnConnected();
   }
 }
 
 void RdpClientWindow::NotifyDisconnected() {
   if (event_handler_) {
     EventHandler* event_handler = event_handler_;
     event_handler_ = nullptr;
     event_handler->OnDisconnected();
   }
 }
 
-void RdpClientWindow::UpdateDesktopResolution() {
+HRESULT RdpClientWindow::UpdateDesktopResolution() {
   if (!client_9_ || !user_logged_in_) {
-    return;
+    return S_FALSE;
   }
 
   // UpdateSessionDisplaySettings() is poorly documented in MSDN and has a few
   // quirks that should be noted.
   // 1.) This method will only work when the user is logged into their session.
   // 2.) The method may return E_UNEXPECTED until some amount of time (seconds)
   //     have elapsed after logging in to the user's session.
-  // TODO(joedow): Investigate adding retry logic for applying the settings.
-  HRESULT result = client_9_->UpdateSessionDisplaySettings(
+  return client_9_->UpdateSessionDisplaySettings(
       screen_resolution_.dimensions().width(),
       screen_resolution_.dimensions().height(),
       screen_resolution_.dimensions().width(),
       screen_resolution_.dimensions().height(),
       /*ulOrientation=*/0,
       screen_resolution_.dpi().x(),
       screen_resolution_.dpi().y());
-  if (FAILED(result)) {
-    LOG(ERROR) << "UpdateSessionDisplaySettings() failed: 0x" << std::hex
-               << result << std::dec;
+}
+
+void RdpClientWindow::ReapplyDesktopResolution() {
+  DCHECK_LT(apply_resolution_attempts_, kMaxResolutionReapplyAttempts);
+
+  HRESULT result = UpdateDesktopResolution();
+  apply_resolution_attempts_++;
+
+  if (SUCCEEDED(result)) {
+    // Successfully applied the new resolution so stop the retry timer.
+    apply_resolution_timer_.Stop();
+  } else if (apply_resolution_attempts_ == kMaxResolutionReapplyAttempts) {
+    // Only log an error on the last attempt to reduce log spam since a few
+    // errors can be expected and don't signal an actual failure.
+    LOG(WARNING) << "All UpdateSessionDisplaySettings() retries failed: 0x"
+                 << std::hex << result;
+    apply_resolution_timer_.Stop();
   }
 }
 
 scoped_refptr<RdpClientWindow::WindowHook>
 RdpClientWindow::WindowHook::Create() {
   scoped_refptr<WindowHook> window_hook = g_window_hook.Pointer()->Get();
 
   if (!window_hook.get()) {
     window_hook = new WindowHook();
   }
@@ skipping 32 matching lines @@
     int code, WPARAM wparam, LPARAM lparam) {
   // Get the hook handle.
   HHOOK hook = g_window_hook.Pointer()->Get()->hook_;
 
   if (code != HCBT_ACTIVATE) {
     return CallNextHookEx(hook, code, wparam, lparam);
   }
 
   // Close the window once all pending window messages are processed.
   HWND window = reinterpret_cast<HWND>(wparam);
-  LOG(WARNING) << "RDP: closing a window: " << std::hex << window << std::dec;
+  LOG(WARNING) << "RDP: closing a window: " << std::hex << window;
   ::PostMessage(window, WM_CLOSE, 0, 0);
   return 0;
 }
 
 }  // namespace remoting