Use same DesktopCaptureOptions in DesktopCaptureBase and DesktopCaptureDevice

chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc

Index: chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
diff --git a/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc b/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
index 05ca164a550c0f16d4e3ea44a94c6e6a1acd4ce1..2182847ee6bb62ce2d1ed9edfc26942455491bce 100644
--- a/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
+++ b/chrome/browser/extensions/api/desktop_capture/desktop_capture_base.cc
@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -42,6 +43,30 @@ const char kEmptySourcesListError[] =
 DesktopCaptureChooseDesktopMediaFunctionBase::PickerFactory* g_picker_factory =
     NULL;
 
+#if !defined(USE_ASH)
+// Creates a DesktopCaptureOptions with required settings. This function should
+// be consistent with CreateDesktopCaptureOptions() function in
+// desktop_capture_device.cc file at
+// https://cs.chromium.org/chromium/src/content/browser/media/capture/desktop_capture_device.cc

[Sergey Ulanov, 2017/06/29 23:49:01]

Instead of duplicating the code here, can this file just call the same function
that's used in content/browser/media/capture/desktop_capture_device.cc?
Both files are in content/browser, so it should be possible to share code
between them.

[Hzj_jie, 2017/06/29 23:56:27]

It's do a little bit confusing, but one file is in chrome/browser, the other is
in content/browser :)

[Sergey Ulanov, 2017/07/05 19:31:07]

Sorry I missed that. Maybe put this function in webrtc? I think you should still
be able to get the state of DirectX feature using
webrtc::field_trial::IsEnabled(). If that doesn't work then I'd prefer exposing
this function in content/public.

[Hzj_jie, 2017/07/07 01:45:08]

Placing this logic in webrtc seems not a good idea to me. Chromoting does not
rely on Finch at all.
But I can move them into content/public/browser/.

+webrtc::DesktopCaptureOptions CreateDesktopCaptureOptions() {
+  auto options = webrtc::DesktopCaptureOptions::CreateDefault();
+  // Leave desktop effects enabled during WebRTC captures.
+  options.set_disable_effects(false);
+#if defined(OS_WIN)
+  static constexpr base::Feature kDirectXCapturer{
+      "DirectXCapturer",
+      base::FEATURE_ENABLED_BY_DEFAULT};
+  if (base::FeatureList::IsEnabled(kDirectXCapturer)) {
+    options.set_allow_directx_capturer(true);
+    options.set_allow_use_magnification_api(false);
+  } else {
+    options.set_allow_use_magnification_api(true);
+  }
+#endif

[Sergey Ulanov, 2017/06/29 23:49:01]

  // defined(OS_WIN)

[Hzj_jie, 2017/06/29 23:56:27]

Done.

+  return options;
+}
+#endif

[Sergey Ulanov, 2017/06/29 23:49:01]

  // defined(OS_ASH)

[Hzj_jie, 2017/06/29 23:56:27]

Done.

+
 }  // namespace
 
 // static
@@ -119,12 +144,10 @@ bool DesktopCaptureChooseDesktopMediaFunctionBase::Execute(
           screen_list = base::MakeUnique<DesktopMediaListAsh>(
               DesktopMediaID::TYPE_SCREEN);
 #else   // !defined(USE_ASH)
-          webrtc::DesktopCaptureOptions capture_options =
-              webrtc::DesktopCaptureOptions::CreateDefault();
-          capture_options.set_disable_effects(false);
           screen_list = base::MakeUnique<NativeDesktopMediaList>(
-              DesktopMediaID::TYPE_SCREEN,
-              webrtc::DesktopCapturer::CreateScreenCapturer(capture_options));
+              content::DesktopMediaID::TYPE_SCREEN,
+              webrtc::DesktopCapturer::CreateScreenCapturer(
+                  CreateDesktopCaptureOptions()));
 #endif  // !defined(USE_ASH)
         }
         have_screen_list = true;
@@ -149,12 +172,10 @@ bool DesktopCaptureChooseDesktopMediaFunctionBase::Execute(
           // windows) created here cannot share the same DesktopCaptureOptions
           // instance. DesktopCaptureOptions owns X connection, which cannot be
           // used on multiple threads concurrently.
-          webrtc::DesktopCaptureOptions capture_options =
-              webrtc::DesktopCaptureOptions::CreateDefault();
-          capture_options.set_disable_effects(false);
           window_list = base::MakeUnique<NativeDesktopMediaList>(
-              DesktopMediaID::TYPE_WINDOW,
-              webrtc::DesktopCapturer::CreateWindowCapturer(capture_options));
+              content::DesktopMediaID::TYPE_WINDOW,
+              webrtc::DesktopCapturer::CreateWindowCapturer(
+                  CreateDesktopCaptureOptions()));
 #endif  // !defined(USE_ASH)
         }
         have_window_list = true;