0

[document pip] Allow resizeTo()/resizeBy() via user gesture

Currently, we don't allow picture-in-picture windows to use the
resizeTo() and resizeBy() APIs to prevent abuse. This CL adds a user
gesture requirement to those APIs for document picture-in-picture
windows to allow them to use those APIs while limiting the potential
for abuse.

Specification: https://github.com/WICG/document-picture-in-picture/pull/104

Bug: 1354325
Change-Id: I6eabb2e9b8923ec1fc395cb44e3cc00ad674f5da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4980802
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Fr <beaufort.francois@gmail.com>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1230725}
This commit is contained in:
Tommy Steimel
2023-11-29 17:32:08 +00:00
committed by Chromium LUCI CQ
parent b278b8c16a
commit f17e2aa8de
7 changed files with 92 additions and 39 deletions
chrome/browser
third_party/blink
renderer
web_tests
external
wpt
document-picture-in-picture

@ -486,24 +486,6 @@ IN_PROC_BROWSER_TEST_F(DocumentPictureInPictureWindowControllerBrowserTest,
"'documentPictureInPicture' in window"));
}
// Make sure that we cannot set window bounds on the PiP window.
IN_PROC_BROWSER_TEST_F(DocumentPictureInPictureWindowControllerBrowserTest,
CannotSetWindowRect) {
LoadTabAndEnterPictureInPicture(browser());
auto* pip_web_contents = window_controller()->GetChildWebContents();
ASSERT_NE(nullptr, pip_web_contents);
auto* browser_view = static_cast<BrowserView*>(
BrowserWindow::FindBrowserWindowWithWebContents(pip_web_contents));
const gfx::Rect bounds = browser_view->GetBounds();
gfx::Rect different_bounds(bounds.x() + 10, bounds.y() + 10,
bounds.width() / 2, bounds.height() / 2);
ASSERT_NE(bounds, different_bounds);
static_cast<content::WebContentsDelegate*>(browser_view->browser())
->SetContentsBounds(pip_web_contents, different_bounds);
EXPECT_EQ(bounds, browser_view->GetBounds());
}
// Make sure that inner bounds of document PiP windows are not smaller than the
// allowed minimum size.
IN_PROC_BROWSER_TEST_F(DocumentPictureInPictureWindowControllerBrowserTest,

@ -1775,7 +1775,7 @@ void Browser::CloseContents(WebContents* source) {
}
void Browser::SetContentsBounds(WebContents* source, const gfx::Rect& bounds) {
if (is_type_normal() || is_type_picture_in_picture()) {
if (is_type_normal()) {
return;
}

@ -1869,14 +1869,22 @@ void LocalDOMWindow::moveTo(int x, int y) const {
page->GetChromeClient().SetWindowRect(window_rect, *frame);
}
void LocalDOMWindow::resizeBy(int x, int y) const {
void LocalDOMWindow::resizeBy(int x,
int y,
ExceptionState& exception_state) const {
if (!GetFrame() || !GetFrame()->IsOutermostMainFrame() ||
document()->IsPrerendering()) {
return;
}
if (IsPictureInPictureWindow())
return;
if (IsPictureInPictureWindow()) {
if (!LocalFrame::ConsumeTransientUserActivation(GetFrame())) {
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"resizeBy() requires user activation in document picture-in-picture");
return;
}
}
LocalFrame* frame = GetFrame();
Page* page = frame->GetPage();
@ -1889,14 +1897,22 @@ void LocalDOMWindow::resizeBy(int x, int y) const {
page->GetChromeClient().SetWindowRect(update, *frame);
}
void LocalDOMWindow::resizeTo(int width, int height) const {
void LocalDOMWindow::resizeTo(int width,
int height,
ExceptionState& exception_state) const {
if (!GetFrame() || !GetFrame()->IsOutermostMainFrame() ||
document()->IsPrerendering()) {
return;
}
if (IsPictureInPictureWindow())
return;
if (IsPictureInPictureWindow()) {
if (!LocalFrame::ConsumeTransientUserActivation(GetFrame())) {
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"resizeTo() requires user activation in document picture-in-picture");
return;
}
}
LocalFrame* frame = GetFrame();
Page* page = frame->GetPage();

@ -332,8 +332,8 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
void moveBy(int x, int y) const;
void moveTo(int x, int y) const;
void resizeBy(int x, int y) const;
void resizeTo(int width, int height) const;
void resizeBy(int x, int y, ExceptionState&) const;
void resizeTo(int width, int height, ExceptionState&) const;
MediaQueryList* matchMedia(const String&);

@ -130,8 +130,8 @@
// browsing context
[MeasureAs=WindowMove] void moveTo(long x, long y);
[MeasureAs=WindowMove] void moveBy(long x, long y);
[MeasureAs=WindowResize] void resizeTo(long x, long y);
[MeasureAs=WindowResize] void resizeBy(long x, long y);
[MeasureAs=WindowResize, RaisesException] void resizeTo(long x, long y);
[MeasureAs=WindowResize, RaisesException] void resizeBy(long x, long y);
// viewport
[HighEntropy=Direct, MeasureAs=WindowInnerWidth, Replaceable] readonly attribute long innerWidth;

@ -687,10 +687,13 @@ TEST_F(PictureInPictureControllerTestWithWidget,
class PictureInPictureControllerChromeClient
: public RenderingTestChromeClient {
public:
explicit PictureInPictureControllerChromeClient(
DummyPageHolder* dummy_page_holder)
: dummy_page_holder_(dummy_page_holder) {}
PictureInPictureControllerChromeClient() = default;
void set_dummy_page_holder(DummyPageHolder* dummy_page_holder) {
dummy_page_holder_ = dummy_page_holder;
}
// RenderingTestChromeClient:
Page* CreateWindowDelegate(LocalFrame*,
const FrameLoadRequest&,
const AtomicString&,
@ -698,12 +701,13 @@ class PictureInPictureControllerChromeClient
network::mojom::blink::WebSandboxFlags,
const SessionStorageNamespaceId&,
bool& consumed_user_gesture) override {
CHECK(dummy_page_holder_);
return &dummy_page_holder_->GetPage();
}
MOCK_METHOD(void, SetWindowRect, (const gfx::Rect&, LocalFrame&));
private:
raw_ptr<DummyPageHolder, ExperimentalRenderer> dummy_page_holder_;
raw_ptr<DummyPageHolder, ExperimentalRenderer> dummy_page_holder_ = nullptr;
};
// Tests for Picture in Picture with a mockable chrome client. This makes it
@ -714,8 +718,10 @@ class PictureInPictureControllerTestWithChromeClient : public RenderingTest {
public:
void SetUp() override {
chrome_client_ =
MakeGarbageCollected<PictureInPictureControllerChromeClient>(
&dummy_page_holder_);
MakeGarbageCollected<PictureInPictureControllerChromeClient>();
dummy_page_holder_ =
std::make_unique<DummyPageHolder>(gfx::Size(), chrome_client_);
chrome_client_->set_dummy_page_holder(dummy_page_holder_.get());
RenderingTest::SetUp();
}
@ -737,7 +743,7 @@ class PictureInPictureControllerTestWithChromeClient : public RenderingTest {
// ownership of it here so that it outlives the GC'd objects. The client
// cannot own it because it also has a GC root to the client; everything would
// leak if we did so.
DummyPageHolder dummy_page_holder_;
std::unique_ptr<DummyPageHolder> dummy_page_holder_;
};
TEST_F(PictureInPictureControllerTestWithChromeClient,
@ -756,12 +762,39 @@ TEST_F(PictureInPictureControllerTestWithChromeClient,
// The Picture in Picture window's base URL should match the opener.
EXPECT_EQ(GetOpenerURL().GetString(), document->BaseURL().GetString());
// Verify that move* and resize* don't call through to the chrome client.
// Verify that move* doesn't call through to the chrome client.
EXPECT_CALL(GetPipChromeClient(), SetWindowRect(_, _)).Times(0);
document->domWindow()->moveTo(10, 10);
document->domWindow()->moveBy(10, 10);
document->domWindow()->resizeTo(10, 10);
document->domWindow()->resizeBy(10, 10);
testing::Mock::VerifyAndClearExpectations(&GetPipChromeClient());
{
// Verify that resizeTo consumes a user gesture, and so only one of the
// following calls will succeed.
EXPECT_CALL(GetPipChromeClient(), SetWindowRect(_, _));
LocalFrame::NotifyUserActivation(
document->GetFrame(), mojom::UserActivationNotificationType::kTest);
ExceptionState exception_state(
ToScriptStateForMainWorld(document->GetFrame())->GetIsolate(),
ExceptionContextType::kOperationInvoke, "Window", "resizeTo");
document->domWindow()->resizeTo(10, 10, exception_state);
document->domWindow()->resizeTo(20, 20, exception_state);
testing::Mock::VerifyAndClearExpectations(&GetPipChromeClient());
}
{
// Verify that resizeBy consumes a user gesture, and so only one of the
// following calls will succeed.
EXPECT_CALL(GetPipChromeClient(), SetWindowRect(_, _));
LocalFrame::NotifyUserActivation(
document->GetFrame(), mojom::UserActivationNotificationType::kTest);
ExceptionState exception_state(
ToScriptStateForMainWorld(document->GetFrame())->GetIsolate(),
ExceptionContextType::kOperationInvoke, "Window", "resizeBy");
document->domWindow()->resizeBy(10, 10, exception_state);
document->domWindow()->resizeBy(20, 20, exception_state);
testing::Mock::VerifyAndClearExpectations(&GetPipChromeClient());
}
// Make sure that the `document` is not the same as the opener.
EXPECT_NE(document, &GetDocument());

@ -0,0 +1,22 @@
<!DOCTYPE html>
<title>Test that calling resizeTo() or resizeBy() on a document
picture-in-picture window requires user gesture</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<body>
<script>
promise_test(async (t) => {
await test_driver.bless('request PiP window from top window');
const pipWindow = await documentPictureInPicture.requestWindow();
await assert_throws_dom('NotAllowedError', pipWindow.DOMException, () => {
pipWindow.resizeBy(10, 10);
}, 'resizeBy() requires a user gesture for document picture-in-picture');
await assert_throws_dom('NotAllowedError', pipWindow.DOMException, () => {
pipWindow.resizeTo(400, 400);
}, 'resizeTo() requires a user gesture for document picture-in-picture');
});
</script>
</body>