Use same DesktopCaptureOptions in DesktopCaptureBase and DesktopCaptureDevice

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

 // Copyright 2015 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 "chrome/browser/extensions/api/desktop_capture/desktop_capture_base.h"
 
 #include <tuple>
 #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"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/media/webrtc/desktop_media_list_ash.h"
 #include "chrome/browser/media/webrtc/desktop_streams_registry.h"
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/native_desktop_media_list.h"
 #include "chrome/browser/media/webrtc/tab_desktop_media_list.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ skipping 14 matching lines @@
 
 namespace {
 
 const char kInvalidSourceNameError[] = "Invalid source type specified.";
 const char kEmptySourcesListError[] =
     "At least one source type must be specified.";
 
 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
 void DesktopCaptureChooseDesktopMediaFunctionBase::SetPickerFactoryForTests(
     PickerFactory* factory) {
   g_picker_factory = factory;
 }
 
 DesktopCaptureChooseDesktopMediaFunctionBase::
     DesktopCaptureChooseDesktopMediaFunctionBase() {
@@ skipping 57 matching lines @@
         }
         std::unique_ptr<DesktopMediaList> screen_list;
         if (g_picker_factory) {
           screen_list =
               g_picker_factory->CreateMediaList(DesktopMediaID::TYPE_SCREEN);
         } else {
 #if defined(USE_ASH)
           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;
         source_lists.push_back(std::move(screen_list));
         break;
       }
       case api::desktop_capture::DESKTOP_CAPTURE_SOURCE_TYPE_WINDOW: {
         if (have_window_list) {
           continue;
         }
         std::unique_ptr<DesktopMediaList> window_list;
         if (g_picker_factory) {
           window_list =
               g_picker_factory->CreateMediaList(DesktopMediaID::TYPE_WINDOW);
         } else {
 #if defined(USE_ASH)
           window_list = base::MakeUnique<DesktopMediaListAsh>(
               DesktopMediaID::TYPE_WINDOW);
 #else   // !defined(USE_ASH)
           // NativeDesktopMediaList calls the capturers on a background thread.
           // This means that the two DesktopCapturer instances (for screens and
           // 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;
         source_lists.push_back(std::move(window_list));
         break;
       }
       case api::desktop_capture::DESKTOP_CAPTURE_SOURCE_TYPE_TAB: {
         if (base::CommandLine::ForCurrentProcess()->HasSwitch(
                 extensions::switches::kDisableTabForDesktopShare) ||
             have_tab_list) {
@@ skipping 142 matching lines @@
 
 void DesktopCaptureRequestsRegistry::CancelRequest(int process_id,
                                                    int request_id) {
   RequestsMap::iterator it = requests_.find(RequestId(process_id, request_id));
   if (it != requests_.end())
     it->second->Cancel();
 }
 
 
 }  // namespace extensions