[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 0cec84b97e54dc8db0bf28f53e37d5efa74f910e)
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: b83393d0f4038aeaf67f970a024d8101df7348d1-refs/heads/main@{#1002911}
diff --git a/chrome/browser/media/webrtc/region_capture_browsertest.cc b/chrome/browser/media/webrtc/region_capture_browsertest.cc
index a7c8804..baa9ac6 100644
--- a/chrome/browser/media/webrtc/region_capture_browsertest.cc
+++ b/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 @@
kServerCount // Must be last.
};
-enum {
+enum Tab {
kMainTab,
kOtherTab,
kTabCount // Must be last.
@@ -102,6 +107,9 @@
}
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 @@
// detection of JS errors.
class RegionCaptureBrowserTest : public WebRtcTestBase {
public:
+ ~RegionCaptureBrowserTest() override = default;
+
void SetUpInProcessBrowserTestFixture() override {
WebRtcTestBase::SetUpInProcessBrowserTestFixture();
@@ -215,20 +225,18 @@
// 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 @@
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 @@
"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)
diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host.cc b/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
index e8d31bee..bff670e 100644
--- a/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
+++ b/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 @@
}
#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 @@
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 @@
// 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,
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index 1047d87..ce668fb 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -596,6 +596,29 @@
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 @@
}
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())
diff --git a/content/browser/renderer_host/media/video_capture_manager.h b/content/browser/renderer_host/media/video_capture_manager.h
index cc30ec2..34a0e70 100644
--- a/content/browser/renderer_host/media/video_capture_manager.h
+++ b/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 @@
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 @@
// |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;
