Skip to content

Commit

Permalink
pip2: Add NavigationThrottle for Document PiP to cancel navigations
Browse files Browse the repository at this point in the history
This CL adds a navigation throttle to prevent navigations in document
picture-in-picture windows and close the PiP window.

(cherry picked from commit d9e92f2)

Bug: 1455964
Change-Id: I75b918516d12c794be7a048a7dbc3c9d32f1a2d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4629929
Reviewed-by: Fr <beaufort.francois@gmail.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1161837}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4646489
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/branch-heads/5845@{#99}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
steimelchrome authored and Chromium LUCI CQ committed Jun 26, 2023
1 parent b546cdd commit f642bc8
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 0 deletions.
2 changes: 2 additions & 0 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3233,6 +3233,8 @@ source_set("browser") {
"webauth/virtual_fido_discovery_factory.h",

# The Document Picture-in-Picture API is not implemented on Android.
"picture_in_picture/document_picture_in_picture_navigation_throttle.cc",
"picture_in_picture/document_picture_in_picture_navigation_throttle.h",
"picture_in_picture/document_picture_in_picture_window_controller_impl.cc",
"picture_in_picture/document_picture_in_picture_window_controller_impl.h",
]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/browser/picture_in_picture/document_picture_in_picture_navigation_throttle.h"

#include "base/functional/bind.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"

namespace content {

// static
std::unique_ptr<DocumentPictureInPictureNavigationThrottle>
DocumentPictureInPictureNavigationThrottle::MaybeCreateThrottleFor(
NavigationHandle* handle) {
// We prevent the main frame of document picture-in-picture windows from doing
// cross-document navigation.
if (!handle->IsInMainFrame() || handle->IsSameDocument() ||
!handle->GetWebContents() ||
!handle->GetWebContents()->GetPictureInPictureOptions().has_value()) {
return nullptr;
}
return std::make_unique<DocumentPictureInPictureNavigationThrottle>(
base::PassKey<DocumentPictureInPictureNavigationThrottle>(), handle);
}

DocumentPictureInPictureNavigationThrottle::
DocumentPictureInPictureNavigationThrottle(
base::PassKey<DocumentPictureInPictureNavigationThrottle>,
NavigationHandle* handle)
: NavigationThrottle(handle) {}

DocumentPictureInPictureNavigationThrottle::
~DocumentPictureInPictureNavigationThrottle() = default;

NavigationThrottle::ThrottleCheckResult
DocumentPictureInPictureNavigationThrottle::WillStartRequest() {
return ClosePiPWindowAndCancelNavigation();
}

NavigationThrottle::ThrottleCheckResult
DocumentPictureInPictureNavigationThrottle::WillCommitWithoutUrlLoader() {
return ClosePiPWindowAndCancelNavigation();
}

const char* DocumentPictureInPictureNavigationThrottle::GetNameForLogging() {
return "DocumentPictureInPictureNavigationThrottle";
}

NavigationThrottle::ThrottleCheckResult
DocumentPictureInPictureNavigationThrottle::
ClosePiPWindowAndCancelNavigation() {
// We are not allowed to synchronously close the WebContents here, so we must
// do it asynchronously.
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&WebContents::ClosePage,
navigation_handle()->GetWebContents()->GetWeakPtr()));

return CANCEL_AND_IGNORE;
}

} // namespace content
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_BROWSER_PICTURE_IN_PICTURE_DOCUMENT_PICTURE_IN_PICTURE_NAVIGATION_THROTTLE_H_
#define CONTENT_BROWSER_PICTURE_IN_PICTURE_DOCUMENT_PICTURE_IN_PICTURE_NAVIGATION_THROTTLE_H_

#include <memory>

#include "base/types/pass_key.h"
#include "content/public/browser/navigation_throttle.h"

namespace content {

// The DocumentPictureInPictureNavigationThrottle is used to prevent document
// picture-in-picture windows from navigating and closes them when they attempt
// to navigate.
class DocumentPictureInPictureNavigationThrottle : public NavigationThrottle {
public:
static std::unique_ptr<DocumentPictureInPictureNavigationThrottle>
MaybeCreateThrottleFor(NavigationHandle* handle);

DocumentPictureInPictureNavigationThrottle(
base::PassKey<DocumentPictureInPictureNavigationThrottle>,
NavigationHandle* handle);
DocumentPictureInPictureNavigationThrottle(
const DocumentPictureInPictureNavigationThrottle&) = delete;
DocumentPictureInPictureNavigationThrottle& operator=(
const DocumentPictureInPictureNavigationThrottle&) = delete;
~DocumentPictureInPictureNavigationThrottle() override;

// NavigationThrottle:
ThrottleCheckResult WillStartRequest() override;
ThrottleCheckResult WillCommitWithoutUrlLoader() override;
const char* GetNameForLogging() override;

private:
ThrottleCheckResult ClosePiPWindowAndCancelNavigation();
};

} // namespace content

#endif // CONTENT_BROWSER_PICTURE_IN_PICTURE_DOCUMENT_PICTURE_IN_PICTURE_NAVIGATION_THROTTLE_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/browser/renderer_host/render_frame_host_impl.h"
#include "content/public/test/test_renderer_host.h"
#include "content/test/navigation_simulator_impl.h"
#include "content/test/test_web_contents.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace content {

class DocumentPictureInPictureNavigationThrottleTest
: public RenderViewHostTestHarness {
public:
DocumentPictureInPictureNavigationThrottleTest()
: RenderViewHostTestHarness(
base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
DocumentPictureInPictureNavigationThrottleTest(
const DocumentPictureInPictureNavigationThrottleTest&) = delete;
DocumentPictureInPictureNavigationThrottleTest& operator=(
const DocumentPictureInPictureNavigationThrottleTest&) = delete;
~DocumentPictureInPictureNavigationThrottleTest() override = default;
};

TEST_F(DocumentPictureInPictureNavigationThrottleTest,
ClosesPictureInPictureWindowOnNavigationStart) {
// Simulate that we're inside a document picture-in-picture window.
blink::mojom::PictureInPictureWindowOptions options;
static_cast<TestWebContents*>(web_contents())
->SetPictureInPictureOptions(options);

// Simulate a navigation, which should fail.
auto nav_simulator = NavigationSimulatorImpl::CreateRendererInitiated(
GURL("https://example.test/"), main_rfh());
nav_simulator->Start();
EXPECT_TRUE(nav_simulator->HasFailed());

// The RenderFrameHost should be asynchronously asked to close. First,
// RunUntilIdle() to request the close, and then wait out the unload timer via
// FastForwardBy().
task_environment()->RunUntilIdle();
task_environment()->FastForwardBy(base::Milliseconds(500));
EXPECT_TRUE(
static_cast<RenderFrameHostImpl*>(main_rfh())->IsPageReadyToBeClosed());
}

TEST_F(DocumentPictureInPictureNavigationThrottleTest,
SameDocumentNavigationsOkay) {
// Simulate that we're inside a document picture-in-picture window.
blink::mojom::PictureInPictureWindowOptions options;
static_cast<TestWebContents*>(web_contents())
->SetPictureInPictureOptions(options);

// Simulate a same-document navigation, which should succeed.
auto nav_simulator =
NavigationSimulatorImpl::CreateRendererInitiated(GURL("#"), main_rfh());
nav_simulator->CommitSameDocument();
EXPECT_FALSE(nav_simulator->HasFailed());
}

} // namespace content
13 changes: 13 additions & 0 deletions content/browser/renderer_host/navigation_throttle_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/metrics/histogram_functions.h"
#include "base/metrics/metrics_hashes.h"
#include "base/strings/strcat.h"
#include "build/build_config.h"
#include "content/browser/devtools/devtools_instrumentation.h"
#include "content/browser/portal/portal_navigation_throttle.h"
#include "content/browser/preloading/prerender/prerender_navigation_throttle.h"
Expand All @@ -24,6 +25,10 @@
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_source_id.h"

#if !BUILDFLAG(IS_ANDROID)
#include "content/browser/picture_in_picture/document_picture_in_picture_navigation_throttle.h"
#endif // !BUILDFLAG(IS_ANDROID)

namespace content {

namespace {
Expand Down Expand Up @@ -160,6 +165,14 @@ void NavigationThrottleRunner::RegisterNavigationThrottles() {
AddThrottle(
BlockedSchemeNavigationThrottle::CreateThrottleForNavigation(request));

#if !BUILDFLAG(IS_ANDROID)
// Prevent cross-document navigations from document picture-in-picture
// windows.
AddThrottle(
DocumentPictureInPictureNavigationThrottle::MaybeCreateThrottleFor(
request));
#endif // !BUILDFLAG(IS_ANDROID)

AddThrottle(AncestorThrottle::MaybeCreateThrottleFor(request));

// Check for mixed content. This is done after the AncestorThrottle and the
Expand Down
4 changes: 4 additions & 0 deletions content/public/test/web_contents_tester.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ class WebContentsTester {
// Returns the time that was set with SetTabSwitchStartTime, or a null
// TimeTicks if it was never called.
virtual base::TimeTicks GetTabSwitchStartTime() = 0;

// Sets the return value for GetPictureInPictureOptions().
virtual void SetPictureInPictureOptions(
absl::optional<blink::mojom::PictureInPictureWindowOptions> options) = 0;
};

} // namespace content
Expand Down
1 change: 1 addition & 0 deletions content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3195,6 +3195,7 @@ test("content_unittests") {
sources += [
"../browser/devtools/protocol/webauthn_handler_unittest.cc",
"../browser/host_zoom_map_impl_unittest.cc",
"../browser/picture_in_picture/document_picture_in_picture_navigation_throttle_unittest.cc",
"../browser/serial/serial_unittest.cc",
"../browser/speech/endpointer/endpointer_unittest.cc",
"../browser/speech/speech_recognition_engine_unittest.cc",
Expand Down
13 changes: 13 additions & 0 deletions content/test/test_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,14 @@ bool TestWebContents::IsBackForwardCacheSupported() {
return back_forward_cache_supported_;
}

const absl::optional<blink::mojom::PictureInPictureWindowOptions>&
TestWebContents::GetPictureInPictureOptions() const {
if (picture_in_picture_options_.has_value()) {
return picture_in_picture_options_;
}
return WebContentsImpl::GetPictureInPictureOptions();
}

int TestWebContents::AddPrerender(const GURL& url) {
DCHECK(!base::FeatureList::IsEnabled(
blink::features::kPrerender2MemoryControls));
Expand Down Expand Up @@ -552,4 +560,9 @@ base::TimeTicks TestWebContents::GetTabSwitchStartTime() {
return tab_switch_start_time_;
}

void TestWebContents::SetPictureInPictureOptions(
absl::optional<blink::mojom::PictureInPictureWindowOptions> options) {
picture_in_picture_options_ = options;
}

} // namespace content
8 changes: 8 additions & 0 deletions content/test/test_web_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester {

base::TimeTicks GetTabSwitchStartTime() final;

void SetPictureInPictureOptions(
absl::optional<blink::mojom::PictureInPictureWindowOptions> options)
override;

protected:
// The deprecated WebContentsTester still needs to subclass this.
explicit TestWebContents(BrowserContext* browser_context);
Expand Down Expand Up @@ -209,6 +213,8 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester {
void ReattachToOuterWebContentsFrame() override {}
void SetPageFrozen(bool frozen) override;
bool IsBackForwardCacheSupported() override;
const absl::optional<blink::mojom::PictureInPictureWindowOptions>&
GetPictureInPictureOptions() const override;

raw_ptr<RenderViewHostDelegateView> delegate_view_override_;

Expand All @@ -226,6 +232,8 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester {
bool is_page_frozen_;
bool back_forward_cache_supported_ = true;
base::TimeTicks tab_switch_start_time_;
absl::optional<blink::mojom::PictureInPictureWindowOptions>
picture_in_picture_options_;
};

} // namespace content
Expand Down

0 comments on commit f642bc8

Please sign in to comment.