[Capture Handle] Move Capture-Handle exposure away from getSettings

getSettings() should expose constrainable constraints. The Capture-Handle is therefore more appropriate to expose through a dedicated getter. Bug: 1200910 Change-Id: Iba9ba97f9828527acc1ff73bbdd1cf565b9f1ad7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2960271 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Elad Alon <eladalon@chromium.org> Cr-Commit-Position: refs/heads/master@{#892137}
chromium · Jun 14, 2021 · 59f9a5b · 59f9a5b
1 parent c209974
commit 59f9a5b
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 110 deletions.
diff --git a/chrome/browser/media/webrtc/capture_handle_browsertest.cc b/chrome/browser/media/webrtc/capture_handle_browsertest.cc
diff --git a/chrome/test/data/webrtc/capturing_page_embedded.html b/chrome/test/data/webrtc/capturing_page_embedded.html
@@ -19,20 +19,20 @@
         // Blocks until onCaptureHandleChange() unblocks.
       }
 
-      function readCaptureHandleFromSettings() {
+      function readCaptureHandle() {
         if (!capturedVideoTrack) {
           window.domAutomationController.send("error-no-video-track");
           return;
         }
 
-        let settings = capturedVideoTrack.getSettings();
-        if (!settings.captureHandle) {
+        let captureHandle = capturedVideoTrack.getCaptureHandle();
+        if (!captureHandle) {
           window.domAutomationController.send("no-embedded-capture-handle");
           return;
         }
 
         window.domAutomationController.send(
-          JSON.stringify(settings.captureHandle)
+          JSON.stringify(captureHandle)
         );
       }
 
@@ -72,8 +72,8 @@
       window.addEventListener("message", (event) => {
         if (event.data == "start-capture") {
           captureOtherTab();
-        } else if (event.data == "read-settings") {
-          readCaptureHandleFromSettings();
+        } else if (event.data == "read-capture-handle") {
+          readCaptureHandle();
         } else {
           window.domAutomationController.send("unrecognized-message");
         }

diff --git a/chrome/test/data/webrtc/capturing_page_main.html b/chrome/test/data/webrtc/capturing_page_main.html
@@ -23,20 +23,20 @@
         // Blocks until the embedded frame's onCaptureHandleChange() unblocks.
       }
 
-      function readCaptureHandleFromSettings() {
+      function readCaptureHandle() {
         if (!capturedVideoTrack) {
           window.domAutomationController.send("error-no-video-track");
           return;
         }
 
-        let settings = capturedVideoTrack.getSettings();
-        if (!settings.captureHandle) {
+        let captureHandle = capturedVideoTrack.getCaptureHandle();
+        if (!captureHandle) {
           window.domAutomationController.send("no-capture-handle");
           return;
         }
 
         window.domAutomationController.send(
-          JSON.stringify(settings.captureHandle)
+          JSON.stringify(captureHandle)
         );
       }
 
@@ -52,10 +52,10 @@
         // window.domAutomationController.send() called by embedded page.
       }
 
-      function readCaptureHandleFromSettingsInEmbeddedFrame() {
+      function readCaptureHandleInEmbeddedFrame() {
         document
           .getElementById("embedded_frame")
-          .contentWindow.postMessage("read-settings", "*");
+          .contentWindow.postMessage("read-capture-handle", "*");
         // window.domAutomationController.send() called by embedded page.
       }
 

diff --git a/third_party/blink/renderer/modules/mediastream/media_stream_track.cc b/third_party/blink/renderer/modules/mediastream/media_stream_track.cc
@@ -33,7 +33,6 @@
 #include "third_party/blink/public/platform/modules/webrtc/webrtc_logging.h"
 #include "third_party/blink/public/web/modules/mediastream/media_stream_video_source.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/bindings/modules/v8/v8_capture_handle.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_capture_handle_change_event.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_capture_handle_change_event_init.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_double_range.h"
@@ -691,19 +690,27 @@ MediaTrackSettings* MediaStreamTrack::getSettings() const {
     }
     settings->setCursor(value);
   }
-  if (platform_settings.capture_handle.has_value()) {
-    const auto& settings_handle = platform_settings.capture_handle.value();
-    auto* capture_handle = CaptureHandle::Create();
-    if (settings_handle.origin) {
-      capture_handle->setOrigin(settings_handle.origin);
-    }
-    capture_handle->setHandle(settings_handle.handle);
-    settings->setCaptureHandle(capture_handle);
-  }
 
   return settings;
 }
 
+CaptureHandle* MediaStreamTrack::getCaptureHandle() const {
+  MediaStreamTrackPlatform::CaptureHandle platform_capture_handle =
+      component_->GetCaptureHandle();
+
+  if (platform_capture_handle.IsEmpty()) {
+    return nullptr;
+  }
+
+  auto* capture_handle = CaptureHandle::Create();
+  if (platform_capture_handle.origin) {
+    capture_handle->setOrigin(platform_capture_handle.origin);
+  }
+  capture_handle->setHandle(platform_capture_handle.handle);
+
+  return capture_handle;
+}
+
 ScriptPromise MediaStreamTrack::applyConstraints(
     ScriptState* script_state,
     const MediaTrackConstraints* constraints) {

diff --git a/third_party/blink/renderer/modules/mediastream/media_stream_track.h b/third_party/blink/renderer/modules/mediastream/media_stream_track.h
@@ -27,8 +27,10 @@
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_MEDIASTREAM_MEDIA_STREAM_TRACK_H_
 
 #include <memory>
+
 #include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/bindings/modules/v8/v8_capture_handle.h"
 #include "third_party/blink/renderer/modules/event_target_modules.h"
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_descriptor.h"
@@ -86,6 +88,7 @@ class MODULES_EXPORT MediaStreamTrack
   MediaTrackCapabilities* getCapabilities() const;
   MediaTrackConstraints* getConstraints() const;
   MediaTrackSettings* getSettings() const;
+  CaptureHandle* getCaptureHandle() const;
   ScriptPromise applyConstraints(ScriptState*, const MediaTrackConstraints*);
 
   // This function is called when constrains have been successfully applied.

diff --git a/third_party/blink/renderer/modules/mediastream/media_stream_track.idl b/third_party/blink/renderer/modules/mediastream/media_stream_track.idl
@@ -52,5 +52,7 @@ enum MediaStreamTrackState {
     MediaTrackCapabilities getCapabilities();
     MediaTrackConstraints getConstraints();
     MediaTrackSettings getSettings();
+    [RuntimeEnabled=CaptureHandle, MeasureAs=CaptureHandle] CaptureHandle? getCaptureHandle();
+
     [CallWith=ScriptState] Promise<void> applyConstraints(optional MediaTrackConstraints constraints = {});
 };
diff --git a/third_party/blink/renderer/modules/mediastream/media_stream_video_track.cc b/third_party/blink/renderer/modules/mediastream/media_stream_video_track.cc
@@ -701,20 +701,34 @@ void MediaStreamVideoTrack::GetSettings(
     settings.display_surface = info->display_surface;
     settings.logical_surface = info->logical_surface;
     settings.cursor = info->cursor;
-    if (info->capture_handle) {
-      settings.capture_handle.emplace();
-      if (!info->capture_handle->origin.opaque()) {
-        settings.capture_handle->origin =
-            String::FromUTF8(info->capture_handle->origin.Serialize());
-      }
-      settings.capture_handle->handle =
-          WebString::FromUTF16(info->capture_handle->capture_handle);
-    } else {
-      settings.capture_handle = absl::nullopt;
-    }
   }
 }
 
+MediaStreamTrackPlatform::CaptureHandle
+MediaStreamVideoTrack::GetCaptureHandle() {
+  MediaStreamTrackPlatform::CaptureHandle capture_handle;
+
+  const MediaStreamDevice& device = source_->device();
+  if (!device.display_media_info.has_value()) {
+    return capture_handle;
+  }
+  const media::mojom::DisplayMediaInformationPtr& info =
+      device.display_media_info.value();
+
+  if (!info->capture_handle) {
+    return capture_handle;
+  }
+
+  if (!info->capture_handle->origin.opaque()) {
+    capture_handle.origin =
+        String::FromUTF8(info->capture_handle->origin.Serialize());
+  }
+  capture_handle.handle =
+      WebString::FromUTF16(info->capture_handle->capture_handle);
+
+  return capture_handle;
+}
+
 void MediaStreamVideoTrack::OnReadyStateChanged(
     WebMediaStreamSource::ReadyState state) {
   DCHECK_CALLED_ON_VALID_THREAD(main_render_thread_checker_);

diff --git a/third_party/blink/renderer/modules/mediastream/media_stream_video_track.h b/third_party/blink/renderer/modules/mediastream/media_stream_video_track.h
@@ -85,6 +85,7 @@ class MODULES_EXPORT MediaStreamVideoTrack : public MediaStreamTrackPlatform {
       WebMediaStreamTrack::ContentHintType content_hint) override;
   void StopAndNotify(base::OnceClosure callback) override;
   void GetSettings(MediaStreamTrackPlatform::Settings& settings) override;
+  MediaStreamTrackPlatform::CaptureHandle GetCaptureHandle() override;
 
   // Add |sink| to receive state changes on the main render thread and video
   // frames in the |callback| method on the IO-thread.

diff --git a/third_party/blink/renderer/platform/mediastream/media_stream_component.cc b/third_party/blink/renderer/platform/mediastream/media_stream_component.cc
@@ -85,6 +85,12 @@ void MediaStreamComponent::GetSettings(
   platform_track_->GetSettings(settings);
 }
 
+MediaStreamTrackPlatform::CaptureHandle
+MediaStreamComponent::GetCaptureHandle() {
+  DCHECK(platform_track_);
+  return platform_track_->GetCaptureHandle();
+}
+
 void MediaStreamComponent::SetContentHint(
     WebMediaStreamTrack::ContentHintType hint) {
   switch (hint) {

diff --git a/third_party/blink/renderer/platform/mediastream/media_stream_component.h b/third_party/blink/renderer/platform/mediastream/media_stream_component.h
@@ -102,6 +102,7 @@ class PLATFORM_EXPORT MediaStreamComponent final
     platform_track_ = std::move(platform_track);
   }
   void GetSettings(MediaStreamTrackPlatform::Settings&);
+  MediaStreamTrackPlatform::CaptureHandle GetCaptureHandle();
 
   String ToString() const;
 

diff --git a/third_party/blink/renderer/platform/mediastream/media_stream_track_platform.cc b/third_party/blink/renderer/platform/mediastream/media_stream_track_platform.cc
@@ -22,4 +22,9 @@ MediaStreamTrackPlatform::MediaStreamTrackPlatform(bool is_local_track)
 
 MediaStreamTrackPlatform::~MediaStreamTrackPlatform() {}
 
+MediaStreamTrackPlatform::CaptureHandle
+MediaStreamTrackPlatform::GetCaptureHandle() {
+  return MediaStreamTrackPlatform::CaptureHandle();
+}
+
 }  // namespace blink
diff --git a/third_party/blink/renderer/platform/mediastream/media_stream_track_platform.h b/third_party/blink/renderer/platform/mediastream/media_stream_track_platform.h
@@ -19,11 +19,6 @@ class PLATFORM_EXPORT MediaStreamTrackPlatform {
  public:
   enum class FacingMode { kNone, kUser, kEnvironment, kLeft, kRight };
 
-  struct CaptureHandle {
-    String origin;
-    String handle;
-  };
-
   struct Settings {
     bool HasFrameRate() const { return frame_rate >= 0.0; }
     bool HasWidth() const { return width >= 0; }
@@ -61,7 +56,13 @@ class PLATFORM_EXPORT MediaStreamTrackPlatform {
     absl::optional<media::mojom::DisplayCaptureSurfaceType> display_surface;
     absl::optional<bool> logical_surface;
     absl::optional<media::mojom::CursorCaptureType> cursor;
-    absl::optional<CaptureHandle> capture_handle;
+  };
+
+  struct CaptureHandle {
+    bool IsEmpty() const { return origin.IsEmpty() && handle.IsEmpty(); }
+
+    String origin;
+    String handle;
   };
 
   explicit MediaStreamTrackPlatform(bool is_local_track);
@@ -81,6 +82,7 @@ class PLATFORM_EXPORT MediaStreamTrackPlatform {
 
   // TODO(hta): Make method pure virtual when all tracks have the method.
   virtual void GetSettings(Settings& settings) {}
+  virtual CaptureHandle GetCaptureHandle();
 
   bool is_local_track() const { return is_local_track_; }