Reland [Windows Host] Prevent connection hang for hosts with 3D Display Mode enabled

This CL was reverted due to Win7 failures (missing function on DXGI.dll),
apparently remoting_unittests are not run on Win7 as part of the standard set of
CQ trybots which is why this wasn't seen earlier.

The fix is to do a LoadLibrary/GetProcAddress dance to remove the DXGI dependency.

I was able to reproduce this failure on a test Win7 machine and by using the
win_chromium_dbg_ng try-bot.  I then verified the tests were passing using those
two methods and verified the original fix is still valid on my Win10 machine.

BUG=875619

Change-Id: Ie56bb9463117b2785c3340ccf4e4785e2e65235d
Reviewed-on: https://chromium-review.googlesource.com/1181528Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584748}

remoting/host/basic_desktop_environment.cc

@@ -20,6 +20,10 @@
 #include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h"
 #include "third_party/webrtc/modules/desktop_capture/mouse_cursor_monitor.h"
 
+#if defined(OS_WIN)
+#include "remoting/host/win/evaluate_d3d.h"
+#endif
+
 #if defined(USE_X11)
 #include "remoting/host/linux/x11_util.h"
 #endif
@@ -98,6 +102,16 @@ BasicDesktopEnvironment::BasicDesktopEnvironment(
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 #if defined(USE_X11)
   IgnoreXServerGrabs(desktop_capture_options().x_display()->display(), true);
+#elif defined(OS_WIN)
+  // The options passed to this instance are determined by a process running in
+  // Session 0.  Access to DirectX functions in Session 0 is limited so the
+  // results are not guaranteed to be accurate in the desktop context.  Due to
+  // this problem, we need to requery the following method to make sure we are
+  // still safe to use D3D APIs.  Only overwrite the value if it isn't safe to
+  // use D3D APIs as we don't want to re-enable this setting if it was disabled
+  // via an experiment or client flag.
+  if (!IsD3DAvailable())
+    options_.desktop_capture_options()->set_allow_directx_capturer(false);
 #endif
 }
 

remoting/host/desktop_environment_options.cc

@@ -45,14 +45,11 @@ DesktopEnvironmentOptions::operator=(
 void DesktopEnvironmentOptions::Initialize() {
   desktop_capture_options_.set_detect_updated_region(true);
 #if defined (OS_WIN)
-  // Whether DirectX capturer can be enabled depends on various facts, include
-  // also how many applications are using related APIs. WebRTC/DesktopCapturer
-  // will take care of all the details. So the check here only ensures it won't
-  // crash the binary: GetD3DCapability() returns false only when the binary
-  // crashes.
-  if (GetD3DCapability()) {
-    desktop_capture_options_.set_allow_directx_capturer(true);
-  }
+  // Whether DirectX capturer can be enabled depends on various factors,
+  // including how many applications are using related APIs.
+  // WebRTC/DesktopCapturer will take care of those details. This check is used
+  // to ensure D3D APIs are safe to access in the current environment.
+  desktop_capture_options_.set_allow_directx_capturer(IsD3DAvailable());
 #endif
 }
 

remoting/host/evaluate_capability.cc

@@ -19,6 +19,7 @@
 #include "remoting/host/switches.h"
 
 #if defined(OS_WIN)
+#include "remoting/host/win/evaluate_3d_display_mode.h"
 #include "remoting/host/win/evaluate_d3d.h"
 #endif
 
@@ -90,6 +91,9 @@ int EvaluateCapabilityLocally(const std::string& type) {
   if (type == kEvaluateD3D) {
     return EvaluateD3D();
   }
+  if (type == kEvaluate3dDisplayMode) {
+    return Evaluate3dDisplayMode();
+  }
 #endif
 
   return kInvalidCommandLineExitCode;

remoting/host/host_attributes.cc

@@ -19,6 +19,7 @@
 #include "base/win/windows_version.h"
 #include "media/base/win/mf_initializer.h"
 #include "media/gpu/windows/media_foundation_video_encode_accelerator_win.h"
+#include "remoting/host/win/evaluate_3d_display_mode.h"
 #include "remoting/host/win/evaluate_d3d.h"
 #endif
 
@@ -103,7 +104,7 @@ std::string GetHostAttributes() {
   }
 #if defined(OS_WIN)
   {
-    GetD3DCapability(&result);
+    GetD3DCapabilities(&result);
 
     auto version = base::win::GetVersion();
     if (version >= base::win::VERSION_WIN8) {

remoting/host/switches.cc

@@ -25,6 +25,7 @@ const char kEvaluateCapabilitySwitchName[] = "evaluate-type";
 
 #if defined(OS_WIN)
 const char kEvaluateD3D[] = "d3d-support";
+const char kEvaluate3dDisplayMode[] = "3d-display-mode";
 #endif
 
 const char kParentWindowSwitchName[] = "parent-window";

remoting/host/switches.h

@@ -40,6 +40,8 @@ extern const char kEvaluateCapabilitySwitchName[];
 #if defined(OS_WIN)
 // Executes EvaluateD3D() function.
 extern const char kEvaluateD3D[];
+// Executes Evaluate3dDisplayMode() function.
+extern const char kEvaluate3dDisplayMode[];
 #endif
 
 // Used to pass the HWND for the parent process to a child process.

remoting/host/win/BUILD.gn

@@ -82,6 +82,8 @@ source_set("win") {
     "com_security.h",
     "default_audio_device_change_detector.cc",
     "default_audio_device_change_detector.h",
+    "evaluate_3d_display_mode.cc",
+    "evaluate_3d_display_mode.h",
     "evaluate_d3d.cc",
     "evaluate_d3d.h",
     "launch_process_with_token.cc",

remoting/host/win/evaluate_3d_display_mode.cc

+// Copyright 2018 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/evaluate_3d_display_mode.h"
+
+#include <D3DCommon.h>
+#include <comdef.h>
+#include <dxgi1_3.h>
+#include <wrl/client.h>
+
+#include <iostream>
+
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/native_library.h"
+#include "base/scoped_native_library.h"
+#include "base/strings/string_util.h"
+#include "base/win/windows_version.h"
+#include "remoting/host/evaluate_capability.h"
+#include "remoting/host/host_exit_codes.h"
+#include "remoting/host/switches.h"
+
+namespace remoting {
+
+namespace {
+
+constexpr char k3dDisplayModeEnabled[] = "3D-Display-Mode";
+
+typedef HRESULT(WINAPI* CreateDXGIFactory2Function)(UINT Flags,
+                                                    REFIID riid,
+                                                    void** ppFactory);
+
+}  // namespace
+
+int Evaluate3dDisplayMode() {
+  // CreateDXGIFactory2 does not exist prior to Win 8.1 but neither does 3D
+  // display mode.
+  if (base::win::GetVersion() < base::win::VERSION_WIN8_1)
+    return kSuccessExitCode;
+
+  // We can't directly reference CreateDXGIFactory2 is it does not exist on
+  // earlier Windows builds.  Therefore we need a LoadLibrary / GetProcAddress
+  // dance.
+  base::ScopedNativeLibrary library(base::FilePath(L"dxgi.dll"));
+  if (!library.is_valid()) {
+    PLOG(INFO) << "Failed to get DXGI library module.";
+    return kInitializationFailed;
+  }
+
+  CreateDXGIFactory2Function factory_func =
+      reinterpret_cast<CreateDXGIFactory2Function>(
+          library.GetFunctionPointer("CreateDXGIFactory2"));
+  if (!factory_func) {
+    PLOG(INFO) << "Failed to get CreateDXGIFactory2 function handle.";
+    return kInitializationFailed;
+  }
+
+  Microsoft::WRL::ComPtr<IDXGIFactory2> factory;
+  HRESULT hr = factory_func(/*flags=*/0, IID_PPV_ARGS(&factory));
+  if (hr != S_OK) {
+    LOG(WARNING) << "CreateDXGIFactory2 failed: 0x" << std::hex << hr;
+    return kInitializationFailed;
+  }
+
+  if (factory->IsWindowedStereoEnabled())
+    std::cout << k3dDisplayModeEnabled << std::endl;
+
+  return kSuccessExitCode;
+}
+
+bool Get3dDisplayModeEnabled() {
+  std::string output;
+  if (EvaluateCapability(kEvaluate3dDisplayMode, &output) != kSuccessExitCode)
+    return false;
+
+  base::TrimString(output, base::kWhitespaceASCII, &output);
+
+  bool is_3d_display_mode_enabled = (output == k3dDisplayModeEnabled);
+  LOG_IF(INFO, is_3d_display_mode_enabled) << "3D Display Mode enabled.";
+
+  return is_3d_display_mode_enabled;
+}
+
+}  // namespace remoting

remoting/host/win/evaluate_3d_display_mode.h

+// Copyright 2018 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.
+
+#ifndef REMOTING_HOST_WIN_EVALUATE_3D_DISPLAY_MODE_H_
+#define REMOTING_HOST_WIN_EVALUATE_3D_DISPLAY_MODE_H_
+
+#include <string>
+#include <vector>
+
+namespace remoting {
+
+// Evaluates the Stereoscopic 3D capability of the system.
+// When this mode is enabled and we are in curtain mode on Windows, all DX
+// CreateDevice calls take several seconds longer than usual.  The result is
+// that the connection may time out or be unusable so we want to query this
+// setting to determine if we should just use the GDI capturer instead.
+// DO NOT call this method within the host process.
+int Evaluate3dDisplayMode();
+
+// Returns whether 3D Display Mode (a.k.a. Stereoscopic 3D) is enabled.
+// Note: This is an expensive call as it creates a new process and blocks on it.
+bool Get3dDisplayModeEnabled();
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_WIN_EVALUATE_3D_DISPLAY_MODE_H_

remoting/host/win/evaluate_d3d.cc

@@ -13,6 +13,7 @@
 #include "remoting/host/evaluate_capability.h"
 #include "remoting/host/host_exit_codes.h"
 #include "remoting/host/switches.h"
+#include "remoting/host/win/evaluate_3d_display_mode.h"
 #include "third_party/webrtc/modules/desktop_capture/win/dxgi_duplicator_controller.h"
 #include "third_party/webrtc/modules/desktop_capture/win/screen_capturer_win_directx.h"
 
@@ -57,26 +58,55 @@ int EvaluateD3D() {
   return kSuccessExitCode;
 }
 
-bool GetD3DCapability(std::vector<std::string>* result /* = nullptr */) {
+bool BlockD3DCheck() {
+  DWORD console_session = WTSGetActiveConsoleSessionId();
+  DWORD current_session = 0;
+  if (!ProcessIdToSessionId(GetCurrentProcessId(), &current_session)) {
+    PLOG(WARNING) << "ProcessIdToSessionId failed: ";
+  }
+
+  // Session 0 is not curtained as it does not have an interactive desktop.
+  bool is_curtained_session =
+      current_session != 0 && current_session != console_session;
+
+  // Skip D3D checks if we are in a curtained session and 3D Display mode is
+  // enabled.  Attempting to create a D3D device in this scenario takes a long
+  // time which often results in the user being disconnected due to timeouts.
+  // After digging in, it looks like the call to D3D11CreateDevice() is spinning
+  // while enumerating the display drivers.  There isn't a simple fix for this
+  // so instead we should skip the D3D caps check and any other D3D related
+  // calls.  This will mean falling back to the GDI capturer.
+  return is_curtained_session && Get3dDisplayModeEnabled();
+}
+
+bool GetD3DCapabilities(std::vector<std::string>* result) {
+  if (BlockD3DCheck()) {
+    result->push_back(kNoDirectXCapturer);
+    return false;
+  }
+
   std::string d3d_info;
   if (EvaluateCapability(kEvaluateD3D, &d3d_info) != kSuccessExitCode) {
-    if (result) {
     result->push_back(kNoDirectXCapturer);
-    }
     return false;
   }
 
-  if (result) {
-    auto capabilities = base::SplitString(
-        d3d_info,
-        base::kWhitespaceASCII,
-        base::TRIM_WHITESPACE,
+  auto capabilities =
+      base::SplitString(d3d_info, base::kWhitespaceASCII, base::TRIM_WHITESPACE,
                         base::SPLIT_WANT_NONEMPTY);
   for (const auto& capability : capabilities) {
     result->push_back(capability);
   }
-  }
+
   return true;
 }
 
+bool IsD3DAvailable() {
+  if (BlockD3DCheck())
+    return false;
+
+  std::string unused;
+  return (EvaluateCapability(kEvaluateD3D, &unused) == kSuccessExitCode);
+}
+
 }  // namespace remoting

remoting/host/win/evaluate_d3d.h

@@ -10,18 +10,21 @@
 
 namespace remoting {
 
-// Evaluates the D3D capability of the system and outputs the results into
-// stdout.
+// Evaluates the D3D capability of the system and sends the results to stdout.
 // DO NOT call this method within the host process. Only call in an isolated
 // child process. I.e. from EvaluateCapabilityLocally().
 int EvaluateD3D();
 
 // Evaluates the D3D capability of the system in a separate process. Returns
-// true if the process succeeded. The capabilities will be stored in |result| if
-// it's not nullptr.
+// true if the process succeeded. The capabilities will be stored in |result|.
 // Note, this is not a cheap call, it uses EvaluateCapability() internally to
 // spawn a new process, which may take a noticeable amount of time.
-bool GetD3DCapability(std::vector<std::string>* result = nullptr);
+bool GetD3DCapabilities(std::vector<std::string>* result);
+
+// Used to ensure that pulling in the DirectX dependencies and creating a D3D
+// Device does not result in a crash or system instability.
+// Note: This is an expensive call as it creates a new process and blocks on it.
+bool IsD3DAvailable();
 
 }  // namespace remoting