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

This reverts commit ee9ee634.

Reason for revert: Likely broke remoting tests on Win7, https://crbug.com/875619

Original change's description:
> [Windows Host] Prevent connection hang for hosts with 3D Display Mode enabled
> 
> This change prevents a hang when connecting to a Windows machine
> which has curtain mode and 3d display mode enabled.  The root cause
> of the hang is that creating a D3D11 device takes several seconds so
> function calls which used to take a few milliseconds now take 5+
> seconds.  I did not see a mechanism to control this display mode
> programatically so I am adding code which can detect the condition
> and the logic to prevent D3D usage when 3D mode is enabled.
> 
> Of note is the change in BasicDesktopEnvironment where I requery the
> D3D apis.  DesktopEnvironmentOptions are queried in Session 0 and
> then passed on to the Desktop process.  There is a non-obvious
> problem with this as many D3D methods cannot be queried or will
> not return reliable results.  This results in a base set of options
> being set in Session 0 (which includes user / experiment overrides)
> and then passed into a session where D3D will return new values.  In
> this CL, I requery the D3D APIs and update the capturer options if
> they should be disabled.
> 
> BUG=836007
> 
> Change-Id: Ib7a6e61fa7987bd093e93a6100b97cc49e3aff05
> Reviewed-on: https://chromium-review.googlesource.com/1164403
> Commit-Queue: Joe Downing <joedow@chromium.org>
> Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583276}

TBR=jamiewalch@chromium.org,joedow@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 836007
Change-Id: I464ba6eb37560ef6fc687516e9e33682bee4ac7a
Reviewed-on: https://chromium-review.googlesource.com/1180681Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584325}

remoting/host/basic_desktop_environment.cc

@@ -20,10 +20,6 @@
 #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
@@ -102,16 +98,6 @@ 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,11 +45,14 @@ 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 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());
+  // 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);
+  }
 #endif
 }
 

remoting/host/evaluate_capability.cc

@@ -19,7 +19,6 @@
 #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
 
@@ -91,9 +90,6 @@ 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,7 +19,6 @@
 #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
 
@@ -104,7 +103,7 @@ std::string GetHostAttributes() {
   }
 #if defined(OS_WIN)
   {
-    GetD3DCapabilities(&result);
+    GetD3DCapability(&result);
 
     auto version = base::win::GetVersion();
     if (version >= base::win::VERSION_WIN8) {

remoting/host/switches.cc

@@ -25,7 +25,6 @@ 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,8 +40,6 @@ 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,8 +82,6 @@ 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/logging.h"
-#include "base/macros.h"
-#include "base/strings/string_util.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";
-
-}  // namespace
-
-int Evaluate3dDisplayMode() {
-  Microsoft::WRL::ComPtr<IDXGIFactory2> factory;
-  HRESULT hr = CreateDXGIFactory2(/*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,7 +13,6 @@
 #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"
 
@@ -58,55 +57,26 @@ int EvaluateD3D() {
   return kSuccessExitCode;
 }
 
-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;
-  }
-
+bool GetD3DCapability(std::vector<std::string>* result /* = nullptr */) {
   std::string d3d_info;
   if (EvaluateCapability(kEvaluateD3D, &d3d_info) != kSuccessExitCode) {
-    result->push_back(kNoDirectXCapturer);
+    if (result) {
+      result->push_back(kNoDirectXCapturer);
+    }
     return false;
   }
 
-  auto capabilities =
-      base::SplitString(d3d_info, base::kWhitespaceASCII, base::TRIM_WHITESPACE,
-                        base::SPLIT_WANT_NONEMPTY);
-  for (const auto& capability : capabilities) {
-    result->push_back(capability);
+  if (result) {
+    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,21 +10,18 @@
 
 namespace remoting {
 
-// Evaluates the D3D capability of the system and sends the results to stdout.
+// Evaluates the D3D capability of the system and outputs the results into
+// 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|.
+// true if the process succeeded. The capabilities will be stored in |result| if
+// it's not nullptr.
 // 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 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();
+bool GetD3DCapability(std::vector<std::string>* result = nullptr);
 
 }  // namespace remoting