[Reland][Region Capture] Fix blocking of other-tab captures

Ensure the cropped track belongs in the same tab
as the issuer of the Crop() request before cropping.

This is a reland of crrev.com/c/3650512.

(cherry picked from commit 0cec84b)

Bug: 1322873
Change-Id: I05692c3daf74b2d64506a73b85ff5fd40e74c729
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3654228
Commit-Queue: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Jordan Bayles <jophba@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1005388}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3660227
Auto-Submit: Elad Alon <eladalon@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#173}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}

chrome/browser/media/webrtc/region_capture_browsertest.cc

@@ -40,6 +40,11 @@
 namespace {
 
 using content::WebContents;
+using testing::Bool;
+using testing::Combine;
+using testing::TestParamInfo;
+using testing::Values;
+using testing::WithParamInterface;
 
 // TODO(crbug.com/1247761): Add tests that verify excessive calls to
 // produceCropId() yield the empty string.
@@ -67,7 +72,7 @@ enum {
   kServerCount  // Must be last.
 };
 
-enum {
+enum Tab {
   kMainTab,
   kOtherTab,
   kTabCount  // Must be last.
@@ -102,6 +107,9 @@ struct TabInfo {
   }
 
   void StartCaptureFromEmbeddedFrame() {
+    // Bring the tab into focus. This avoids getDisplayMedia rejection.
+    browser->tab_strip_model()->ActivateTabAt(tab_strip_index);
+
     std::string script_result;
     EXPECT_TRUE(content::ExecuteScriptAndExtractString(
         web_contents->GetMainFrame(), "startCaptureFromEmbeddedFrame();",
@@ -155,6 +163,8 @@ struct TabInfo {
 // detection of JS errors.
 class RegionCaptureBrowserTest : public WebRtcTestBase {
  public:
+  ~RegionCaptureBrowserTest() override = default;
+
   void SetUpInProcessBrowserTestFixture() override {
     WebRtcTestBase::SetUpInProcessBrowserTestFixture();
 
@@ -215,20 +225,18 @@ class RegionCaptureBrowserTest : public WebRtcTestBase {
   // Set up all (necessary) tabs, loads iframes, and start capturing the
   // relevant tab.
   void SetUpTest(Frame capturing_entity, bool self_capture) {
-    // Main page (for self-capture).
+    // Other page (for other-tab-capture).
+    SetUpPage("/webrtc/region_capture_other_main.html",
+              servers_[kOtherPageTopLevelDocument].get(),
+              "/webrtc/region_capture_other_embedded.html",
+              servers_[kOtherPageEmbeddedDocument].get(), &tabs_[kOtherTab]);
+
+    // Main page (for self-capture). Instantiate it second to help it get focus.
     SetUpPage("/webrtc/region_capture_main.html",
               servers_[kMainPageTopLevelDocument].get(),
               "/webrtc/region_capture_embedded.html",
               servers_[kMainPageEmbeddedDocument].get(), &tabs_[kMainTab]);
 
-    if (!self_capture) {
-      // Other page (for other-tab-capture).
-      SetUpPage("/webrtc/region_capture_other_main.html",
-                servers_[kOtherPageTopLevelDocument].get(),
-                "/webrtc/region_capture_other_embedded.html",
-                servers_[kOtherPageEmbeddedDocument].get(), &tabs_[kOtherTab]);
-    }
-
     DCHECK(command_line_);
     command_line_->AppendSwitchASCII(
         switches::kAutoSelectTabCaptureSourceByTitle,
@@ -376,28 +384,6 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(tab.CropTo(""), "top-level-crop-success");
 }
 
-IN_PROC_BROWSER_TEST_F(RegionCaptureBrowserTest,
-                       CropToRejectedIfElementInAnotherTabTopLevel) {
-  SetUpTest(Frame::kTopLevelDocument, /*self_capture=*/false);
-
-  const std::string crop_id =
-      tabs_[kOtherTab].ProduceCropId(Frame::kTopLevelDocument);
-  ASSERT_THAT(crop_id, IsValidCropId());
-
-  EXPECT_EQ(tabs_[kMainTab].CropTo(crop_id), "top-level-crop-error");
-}
-
-IN_PROC_BROWSER_TEST_F(RegionCaptureBrowserTest,
-                       CropToRejectedIfElementInAnotherTabEmbeddedFrame) {
-  SetUpTest(Frame::kTopLevelDocument, /*self_capture=*/false);
-
-  const std::string crop_id =
-      tabs_[kOtherTab].ProduceCropId(Frame::kEmbeddedFrame);
-  ASSERT_THAT(crop_id, IsValidCropId());
-
-  EXPECT_EQ(tabs_[kMainTab].CropTo(crop_id), "top-level-crop-error");
-}
-
 IN_PROC_BROWSER_TEST_F(RegionCaptureBrowserTest, MaxCropIdsInTopLevelDocument) {
   SetUpTest(Frame::kNone, /*self_capture=*/false);
   TabInfo& tab = tabs_[kMainTab];
@@ -495,4 +481,85 @@ IN_PROC_BROWSER_TEST_F(RegionCaptureBrowserTest,
             "embedded-produce-crop-id-error");
 }
 
+// Suite of tests ensuring that only self-capture may crop, and that it may
+// only crop to targets in its own tab, but that any target in its own tab
+// is permitted.
+class RegionCaptureSelfCaptureOnlyBrowserTest
+    : public RegionCaptureBrowserTest,
+      public WithParamInterface<std::tuple<Frame, bool, Tab, Frame>> {
+ public:
+  RegionCaptureSelfCaptureOnlyBrowserTest()
+      : capturing_entity_(std::get<0>(GetParam())),
+        self_capture_(std::get<1>(GetParam())),
+        target_element_tab_(std::get<2>(GetParam())),
+        target_frame_(std::get<3>(GetParam())) {}
+  ~RegionCaptureSelfCaptureOnlyBrowserTest() override = default;
+
+ protected:
+  // The capture is done from kMainTab in all instances of this parameterized
+  // test. |capturing_entity_| controls whether the capture is initiated
+  // from the top-level document of said tab, or an embedded frame.
+  const Frame capturing_entity_;
+
+  // Whether capturing self, or capturing the other tab.
+  const bool self_capture_;
+
+  // Whether the element on whose crop-ID we'll call cropTo():
+  // * |target_element_tab_| - whether it's in kMainTab or in kOtherTab.
+  // * |target_frame_| - whether it's in the top-level or an embedded frame.
+  const Tab target_element_tab_;
+  const Frame target_frame_;  // Top-level or embedded frame.
+};
+
+std::string ParamsToString(
+    const TestParamInfo<RegionCaptureSelfCaptureOnlyBrowserTest::ParamType>&
+        info) {
+  return base::StrCat(
+      {std::get<0>(info.param) == Frame::kTopLevelDocument ? "TopLevel"
+                                                           : "EmbeddedFrame",
+       std::get<1>(info.param) ? "SelfCapturing" : "CapturingOtherTab",
+       "AndCroppingToElementIn",
+       std::get<2>(info.param) == kMainTab ? "OwnTabs" : "OtherTabs",
+       std::get<3>(info.param) == Frame::kTopLevelDocument ? "TopLevel"
+                                                           : "EmbeddedFrame"});
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    _,
+    RegionCaptureSelfCaptureOnlyBrowserTest,
+    Combine(Values(Frame::kTopLevelDocument, Frame::kEmbeddedFrame),
+            Bool(),
+            Values(kMainTab, kOtherTab),
+            Values(Frame::kTopLevelDocument, Frame::kEmbeddedFrame)),
+    ParamsToString);
+
+IN_PROC_BROWSER_TEST_P(RegionCaptureSelfCaptureOnlyBrowserTest, CropTo) {
+  SetUpTest(capturing_entity_, self_capture_);
+
+  // Prevent test false-positive - ensure that both tabs participating in the
+  // test have at least one associated crop-ID, or otherwise they would not
+  // have a CropIdWebContentsHelper.
+  // To make things even clearer, ensure both the top-level and the embedded
+  // frame have produced crop-IDs. (This should not be necessary, but is
+  // done as an extra buffer against false-positives.)
+  tabs_[kMainTab].ProduceCropId(Frame::kTopLevelDocument);
+  tabs_[kMainTab].ProduceCropId(Frame::kEmbeddedFrame);
+  tabs_[kOtherTab].ProduceCropId(Frame::kTopLevelDocument);
+  tabs_[kOtherTab].ProduceCropId(Frame::kEmbeddedFrame);
+
+  const std::string crop_id =
+      tabs_[target_element_tab_].ProduceCropId(target_frame_);
+  ASSERT_THAT(crop_id, IsValidCropId());
+
+  // Cropping only permitted if both conditions hold.
+  const bool expect_permitted =
+      (self_capture_ && target_element_tab_ == kMainTab);
+
+  const std::string expected_result = base::StrCat(
+      {capturing_entity_ == Frame::kTopLevelDocument ? "top-level" : "embedded",
+       "-", expect_permitted ? "crop-success" : "crop-error"});
+
+  EXPECT_EQ(tabs_[kMainTab].CropTo(crop_id), expected_result);
+}
+
 #endif  //  !BUILDFLAG(IS_CHROMEOS_LACROS)

content/browser/renderer_host/media/media_stream_dispatcher_host.cc

@@ -17,6 +17,7 @@
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -68,26 +69,40 @@ StartObservingWebContents(int render_process_id,
 }
 
 #if !BUILDFLAG(IS_ANDROID)
+// Helper for getting the top-level WebContents associated with a given ID.
+// Returns nullptr if one does not exist (e.g. has gone away).
+WebContents* GetMainFrameWebContents(const GlobalRoutingID& global_routing_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  if (global_routing_id == GlobalRoutingID()) {
+    return nullptr;
+  }
+
+  RenderFrameHost* const rfh = RenderFrameHost::FromID(
+      global_routing_id.child_id, global_routing_id.route_id);
+  return rfh ? WebContents::FromRenderFrameHost(rfh->GetMainFrame()) : nullptr;
+}
+
 // Checks whether a track living in the WebContents indicated by
 // (render_process_id, render_frame_id) may be cropped to the crop-target
 // indicated by |crop_id|.
-bool IsCropTargetValid(int render_process_id,
-                       int render_frame_id,
-                       const base::Token& crop_id) {
-  RenderFrameHost* const rfh =
-      RenderFrameHost::FromID(render_process_id, render_frame_id);
-  if (!rfh) {
+bool MayCrop(const GlobalRoutingID& capturing_id,
+             const GlobalRoutingID& captured_id,
+             const base::Token& crop_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  WebContents* const capturing_wc = GetMainFrameWebContents(capturing_id);
+  if (!capturing_wc) {
     return false;
   }
 
-  WebContents* const web_contents =
-      WebContents::FromRenderFrameHost(rfh->GetMainFrame());
-  if (!web_contents) {
+  WebContents* const captured_wc = GetMainFrameWebContents(captured_id);
+  if (capturing_wc != captured_wc) {  // Null or not-same-tab.
     return false;
   }
 
   CropIdWebContentsHelper* const helper =
-      CropIdWebContentsHelper::FromWebContents(web_contents);
+      CropIdWebContentsHelper::FromWebContents(captured_wc);
   if (!helper) {
     // No crop-IDs were ever produced on this WebContents.
     // Any non-zero crop-ID should be rejected on account of being
@@ -531,6 +546,10 @@ void MediaStreamDispatcherHost::Crop(const base::UnguessableToken& device_id,
                                      CropCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  const GlobalRoutingID captured_id =
+      media_stream_manager_->video_capture_manager()->GetGlobalRoutingID(
+          device_id);
+
   // Hop to the UI thread to verify that cropping to |crop_id| is permitted
   // from this particular context. Namely, cropping is currently only allowed
   // for self-capture, so the crop_id has to be associated with the top-level
@@ -539,8 +558,9 @@ void MediaStreamDispatcherHost::Crop(const base::UnguessableToken& device_id,
   // when SelfOwnedReceiver properly supports this.
   GetUIThreadTaskRunner({})->PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::BindOnce(&IsCropTargetValid, render_process_id_, render_frame_id_,
-                     crop_id),
+      base::BindOnce(&MayCrop,
+                     GlobalRoutingID(render_process_id_, render_frame_id_),
+                     captured_id, crop_id),
       base::BindOnce(&MediaStreamDispatcherHost::OnCropValidationComplete,
                      weak_factory_.GetWeakPtr(), device_id, crop_id,
                      crop_version,

content/browser/renderer_host/media/video_capture_manager.cc

@@ -596,6 +596,29 @@ VideoCaptureManager::GetDeviceFormatInUse(
   return device_in_use ? device_in_use->GetVideoCaptureFormat() : absl::nullopt;
 }
 
+GlobalRoutingID VideoCaptureManager::GetGlobalRoutingID(
+    const base::UnguessableToken& session_id) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  VideoCaptureController* const controller =
+      LookupControllerBySessionId(session_id);
+  if (!controller || !controller->IsDeviceAlive() ||
+      !blink::IsVideoDesktopCaptureMediaType(controller->stream_type())) {
+    return GlobalRoutingID();
+  }
+
+  const DesktopMediaID desktop_media_id =
+      DesktopMediaID::Parse(controller->device_id());
+
+  if (desktop_media_id.type != DesktopMediaID::Type::TYPE_WEB_CONTENTS ||
+      desktop_media_id.web_contents_id.is_null()) {
+    return GlobalRoutingID();
+  }
+
+  return GlobalRoutingID(desktop_media_id.web_contents_id.render_process_id,
+                         desktop_media_id.web_contents_id.main_render_frame_id);
+}
+
 void VideoCaptureManager::SetDesktopCaptureWindowId(
     const media::VideoCaptureSessionId& session_id,
     gfx::NativeViewId window_id) {
@@ -798,7 +821,7 @@ void VideoCaptureManager::DestroyControllerIfNoClients(
 }
 
 VideoCaptureController* VideoCaptureManager::LookupControllerBySessionId(
-    const base::UnguessableToken& session_id) {
+    const base::UnguessableToken& session_id) const {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   SessionMap::const_iterator session_it = sessions_.find(session_id);
   if (session_it == sessions_.end())

content/browser/renderer_host/media/video_capture_manager.h

@@ -26,6 +26,7 @@
 #include "content/browser/renderer_host/media/video_capture_device_launch_observer.h"
 #include "content/browser/renderer_host/media/video_capture_provider.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/screenlock_observer.h"
 #include "media/base/video_facing.h"
 #include "media/capture/video/video_capture_device.h"
@@ -178,6 +179,12 @@ class CONTENT_EXPORT VideoCaptureManager
       blink::mojom::MediaStreamType stream_type,
       const std::string& device_id);
 
+  // If there is a capture session associated with |session_id|, and the
+  // captured entity a tab, return the GlobalRoutingID of the captured tab.
+  // Otherwise, returns an empty GlobalRoutingID.
+  GlobalRoutingID GetGlobalRoutingID(
+      const base::UnguessableToken& session_id) const;
+
   // Sets the platform-dependent window ID for the desktop capture notification
   // UI for the given session.
   void SetDesktopCaptureWindowId(const media::VideoCaptureSessionId& session_id,
@@ -251,7 +258,7 @@ class CONTENT_EXPORT VideoCaptureManager
   // |device_id| and |type| (if it is already opened), by its |controller| or by
   // its |serial_id|. In all cases, if not found, nullptr is returned.
   VideoCaptureController* LookupControllerBySessionId(
-      const base::UnguessableToken& session_id);
+      const base::UnguessableToken& session_id) const;
   VideoCaptureController* LookupControllerByMediaTypeAndDeviceId(
       blink::mojom::MediaStreamType type,
       const std::string& device_id) const;