[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}

chrome/browser/picture_in_picture/document_picture_in_picture_window_controller_browsertest.cc

@@ -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,

chrome/browser/ui/browser.cc

@@ -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;
   }
 

third_party/blink/renderer/core/frame/local_dom_window.cc

@@ -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())
+  if (IsPictureInPictureWindow()) {
-    return;
+    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())
+  if (IsPictureInPictureWindow()) {
-    return;
+    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();

third_party/blink/renderer/core/frame/local_dom_window.h

@@ -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 resizeBy(int x, int y, ExceptionState&) const;
-  void resizeTo(int width, int height) const;
+  void resizeTo(int width, int height, ExceptionState&) const;
 
   MediaQueryList* matchMedia(const String&);
 

third_party/blink/renderer/core/frame/window.idl

@@ -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, RaisesException] void resizeTo(long x, long y);
-    [MeasureAs=WindowResize] void resizeBy(long x, long y);
+    [MeasureAs=WindowResize, RaisesException] void resizeBy(long x, long y);
 
     // viewport
     [HighEntropy=Direct, MeasureAs=WindowInnerWidth, Replaceable] readonly attribute long innerWidth;

third_party/blink/renderer/modules/document_picture_in_picture/picture_in_picture_controller_test.cc

@@ -687,10 +687,13 @@ TEST_F(PictureInPictureControllerTestWithWidget,
 class PictureInPictureControllerChromeClient
     : public RenderingTestChromeClient {
  public:
-  explicit PictureInPictureControllerChromeClient(
+  PictureInPictureControllerChromeClient() = default;
-      DummyPageHolder* dummy_page_holder)
-      : dummy_page_holder_(dummy_page_holder) {}
 
+  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>(
+        MakeGarbageCollected<PictureInPictureControllerChromeClient>();
-            &dummy_page_holder_);
+    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);
+  testing::Mock::VerifyAndClearExpectations(&GetPipChromeClient());
-  document->domWindow()->resizeBy(10, 10);
+
+  {
+    // 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());

third_party/blink/web_tests/external/wpt/document-picture-in-picture/resize-requires-user-gesture.https.html

@@ -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>