[Chromoting] Use latest API to check whether DX capturer is supported

remoting/host/host_attributes.cc

Index: remoting/host/host_attributes.cc
diff --git a/remoting/host/host_attributes.cc b/remoting/host/host_attributes.cc
index 307d04e289fd66c4df9a4ed1219155dcf2904287..24a11470d7ae3bc02eb117781b4c043c9c0dd261 100644
--- a/remoting/host/host_attributes.cc
+++ b/remoting/host/host_attributes.cc
@@ -13,6 +13,7 @@
 #include "base/atomicops.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/win/windows_version.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
@@ -23,6 +24,7 @@
 namespace remoting {
 
 namespace {
+
 static constexpr char kSeparator[] = ",";
 
 struct Attribute {
@@ -30,6 +32,14 @@ struct Attribute {
   bool(* get_value_func)();
 };
 
+void AppendAttribute(std::string* result, const char* attribute) {

[Do not use (sergeyu), 2017/06/27 05:54:44]

It would be cleaner to collect all attributes into a vector<StringPiece> and
then join them together using base::JoinString(). 

[Hzj_jie, 2017/06/27 23:03:06]

Done.

+  DCHECK_EQ(std::string(attribute).find(kSeparator), std::string::npos);
+  if (!result->empty()) {
+    result->append(kSeparator);
+  }
+  result->append(attribute);
+}
+
 inline constexpr bool IsDebug() {
 #if defined(NDEBUG)
   return false;
@@ -65,27 +75,32 @@ inline constexpr bool IsNonOfficialBuild() {
 }
 
 #if defined(OS_WIN)
-inline bool MinD3DFeatureLevelGreatThan10() {
-  webrtc::DxgiDuplicatorController::D3dInfo info;
-  if (webrtc::DxgiDuplicatorController::Instance()->RetrieveD3dInfo(&info)) {
-    return info.min_feature_level >= D3D_FEATURE_LEVEL_10_0;
+// TODO(zijiehe): Use ScreenCapturerWinDirectx::IsSessionUnsupported().
+bool IsRunningInSession0() {
+  DWORD session_id = 0;
+  if (!::ProcessIdToSessionId(::GetCurrentProcessId(), &session_id)) {
+    LOG(WARNING) << "Failed to retrieve current session id.";

[joedow, 2017/06/27 15:44:58]

If you use PLOG here, the platform error code will also be logged.

[Hzj_jie, 2017/06/27 23:03:06]

Done.

+    // Assume the binary is running in session 0 if the session id cannot be
+    // retrieved, so IsDirectxCapturerSupported() will fallback to use Windows
+    // version.
+    return true;
   }
-  return false;
+  return session_id == 0;
 }
 
-inline bool MinD3DFeatureLevelGreatThan11() {
-  webrtc::DxgiDuplicatorController::D3dInfo info;
-  if (webrtc::DxgiDuplicatorController::Instance()->RetrieveD3dInfo(&info)) {
-    return info.min_feature_level >= D3D_FEATURE_LEVEL_11_0;
+bool IsDirectxCapturerSupported() {
+  if (webrtc::ScreenCapturerWinDirectx::IsSupported()) {

[Do not use (sergeyu), 2017/06/27 05:54:44]

Is there anything that we get by calling ScreenCapturerWinDirectx::IsSupported()
here? Would it be simpler to just always use windows version? otherwise the
result of GetHostAttributes() is not consistent across the host processes.

[Hzj_jie, 2017/06/27 23:03:06]

I believe this used to work well before some upgrade of my Windows 10 machine.
Or maybe we can return Windows version in the HostAttributes to avoid the
confusion: DirectX capturer will fallback to GDI capturer if the initialization
failed anyway. So if we cannot rely on the
ScreenCapturerWinDirectx::IsSupported() in session 0 anymore, using Windows
version does not really introduce new biases.

+    return true;
   }
-  return false;
-}
 
-inline bool MinD3DFeatureLevelGreatThan12() {
-  webrtc::DxgiDuplicatorController::D3dInfo info;
-  if (webrtc::DxgiDuplicatorController::Instance()->RetrieveD3dInfo(&info)) {
-    return info.min_feature_level >= D3D_FEATURE_LEVEL_12_0;
+  if (IsRunningInSession0()) {
+    LOG(WARNING) << "Current binary is running in session 0. DXGI components "
+                    "cannot be initialized. Fallback to use Windows version to "
+                    "decide the support of DirectX capturer. It may be "
+                    "inaccurate.";

[joedow, 2017/06/27 15:44:58]

IIUC this method is going to be called in session 0 every time a connection is
made (since the host will need to query it to send its DX support info to the
client).  If so, then this warning isn't very useful.  We can downgrade it to a
VLOG or LOG(INFO) as nothing is going wrong, it is the expected behavior for
session 0.

[Hzj_jie, 2017/06/27 23:03:06]

Done.

+    return base::win::GetVersion() >= base::win::VERSION_WIN8;
   }
+
   return false;
 }
 #endif
@@ -107,10 +122,7 @@ static constexpr Attribute kAttributes[] = {
   { "OfficialBuild", &IsOfficialBuild },
   { "NonOfficialBuild", &IsNonOfficialBuild },
 #if defined(OS_WIN)
-  { "DirectX-Capturer", &webrtc::ScreenCapturerWinDirectx::IsSupported },
-  { "MinD3DGT10", &MinD3DFeatureLevelGreatThan10 },
-  { "MinD3DGT11", &MinD3DFeatureLevelGreatThan11 },
-  { "MinD3DGT12", &MinD3DFeatureLevelGreatThan12 },
+  { "DirectX-Capturer", &IsDirectxCapturerSupported },
 #endif
 };
 
@@ -119,6 +131,11 @@ static constexpr Attribute kAttributes[] = {
 static_assert(std::is_pod<Attribute>::value, "Attribute should be POD.");
 
 std::string GetHostAttributes() {
+#if defined(OS_WIN)
+  // Ensure the following ScreenCapturerWinDirectx contructions won't

[Do not use (sergeyu), 2017/06/27 05:54:44]

This is still confusing. Remove IsDirectxCapturerSupported() and handle DX
support same as DX version attributes? that would allow to have only one ifdef
for Windows in this function.

[Hzj_jie, 2017/06/27 23:03:06]

In that case, I would prefer to return Windows version instead of
DirectX-Capturer to avoid any confusing.
Windows 8 requires only video cards that support DirectX 9, but DirectX capturer
requires DirectX 11. So using Windows version is inaccurate anyway: naming the
attribute as DirectX-Capturer is improper here.

+  // initialize and unload DxgiDuplicatorController.
+  auto controller = webrtc::DxgiDuplicatorController::Instance();
+#endif
   std::string result;
   // By using ranged for-loop, MSVC throws error C3316:
   // 'const remoting::StaticAttribute [0]':
@@ -127,12 +144,26 @@ std::string GetHostAttributes() {
     const auto& attribute = kAttributes[i];
     DCHECK_EQ(std::string(attribute.name).find(kSeparator), std::string::npos);
     if (attribute.get_value_func()) {
-      if (!result.empty()) {
-        result.append(kSeparator);
-      }
-      result.append(attribute.name);
+      AppendAttribute(&result, attribute.name);
+    }
+  }
+#if defined(OS_WIN)
+  {
+    webrtc::DxgiDuplicatorController::D3dInfo info;
+    webrtc::ScreenCapturerWinDirectx::RetrieveD3dInfo(&info);
+    if (info.min_feature_level >= D3D_FEATURE_LEVEL_10_0) {
+      AppendAttribute(&result, "MinD3DGT10");
+    }
+    if (info.min_feature_level >= D3D_FEATURE_LEVEL_11_0) {
+      AppendAttribute(&result, "MinD3DGT11");
+    }
+    if (info.min_feature_level >= D3D_FEATURE_LEVEL_12_0) {
+      AppendAttribute(&result, "MinD3DGT12");
     }
   }
+  // Avoid the warning of unused variable.
+  controller = nullptr;
+#endif
 
   return result;
 }