Skip to content

Commit

Permalink
Introduce per browsing context group page pauser
Browse files Browse the repository at this point in the history
Existing ScopedPagePauser pauses all pages. This behavior can be
problematic when a renderer process hosts two same origin tabs
with different browsing context groups: If one tab pauses pages, the
pages belong to the other tab are also paused unexpectedly.

To avoid such situation, this CL introduces a page pauser that only
pauses pages that belong to the given browsing context group. The new
pauser is behind a flag and the flag is disabled by default, as we
also need to update other components such as scheduler so that these
are aware of per browsing context group pauser.

Bug: 1475531

Change-Id: I6a7b8935bb66ccde577e856e3bdcb05e613afced
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4803088
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188137}
  • Loading branch information
bashi authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent 7e67cd4 commit 05e9df2
Show file tree
Hide file tree
Showing 11 changed files with 243 additions and 12 deletions.
4 changes: 4 additions & 0 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,10 @@ BASE_FEATURE(kScopeMemoryCachePerContext,
"ScopeMemoryCachePerContext",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kPausePagesPerBrowsingContextGroup,
"PausePagesPerBrowsingContextGroup",
base::FEATURE_DISABLED_BY_DEFAULT);

// Controls script streaming.
BASE_FEATURE(kScriptStreaming,
"ScriptStreaming",
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,10 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kSaveDataImgSrcset);

BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kScopeMemoryCachePerContext);

// When enabled, only pages that belong to a certain browsing context group are
// paused instead of all pages.
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kPausePagesPerBrowsingContextGroup);

BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kScriptStreaming);

// If enabled, parser-blocking scripts are loaded asynchronously. The target
Expand Down
7 changes: 5 additions & 2 deletions third_party/blink/public/platform/web_scoped_page_pauser.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,23 @@

namespace blink {

class WebLocalFrameImpl;

// WebScopedPagePauser implements the concept of 'pause' in HTML standard.
// https://html.spec.whatwg.org/C/#pause
// All script execution is suspended while any of WebScopedPagePauser instances
// exists.
class WebScopedPagePauser {
public:
BLINK_EXPORT static std::unique_ptr<WebScopedPagePauser> Create();
BLINK_EXPORT static std::unique_ptr<WebScopedPagePauser> Create(
WebLocalFrameImpl&);

WebScopedPagePauser(const WebScopedPagePauser&) = delete;
WebScopedPagePauser& operator=(const WebScopedPagePauser&) = delete;
BLINK_EXPORT ~WebScopedPagePauser();

private:
WebScopedPagePauser();
explicit WebScopedPagePauser(WebLocalFrameImpl&);
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class ClientMessageLoopAdapter : public MainThreadDebugger::ClientMessageLoop {
view->GetChromeClient().NotifyPopupOpeningObservers();

// 2. Disable active objects
page_pauser_ = WebScopedPagePauser::Create();
page_pauser_ = WebScopedPagePauser::Create(*frame);

// 3. Process messages until quitNow is called.
message_loop_->Run();
Expand Down
38 changes: 32 additions & 6 deletions third_party/blink/renderer/core/exported/web_scoped_page_pauser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

#include "third_party/blink/public/platform/web_scoped_page_pauser.h"

#include "base/feature_list.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/core/page/scoped_browsing_context_group_pauser.h"
#include "third_party/blink/renderer/core/page/scoped_page_pauser.h"

namespace blink {
Expand All @@ -19,20 +23,42 @@ Vector<std::unique_ptr<ScopedPagePauser>>& PagePauserStack() {
return pauser_stack;
}

Vector<std::unique_ptr<ScopedBrowsingContextGroupPauser>>&
BrowsingContextGroupPauserStack() {
DEFINE_STATIC_LOCAL(Vector<std::unique_ptr<ScopedBrowsingContextGroupPauser>>,
pauser_stack, ());
return pauser_stack;
}

} // namespace

// static
std::unique_ptr<WebScopedPagePauser> WebScopedPagePauser::Create() {
return std::unique_ptr<WebScopedPagePauser>(new WebScopedPagePauser);
std::unique_ptr<WebScopedPagePauser> WebScopedPagePauser::Create(
WebLocalFrameImpl& frame) {
return std::unique_ptr<WebScopedPagePauser>(new WebScopedPagePauser(frame));
}

WebScopedPagePauser::WebScopedPagePauser() {
PagePauserStack().push_back(std::make_unique<ScopedPagePauser>());
WebScopedPagePauser::WebScopedPagePauser(WebLocalFrameImpl& frame) {
if (base::FeatureList::IsEnabled(
features::kPausePagesPerBrowsingContextGroup)) {
Page* page = WebFrame::ToCoreFrame(frame)->GetPage();
CHECK(page);
BrowsingContextGroupPauserStack().push_back(
std::make_unique<ScopedBrowsingContextGroupPauser>(*page));
} else {
PagePauserStack().push_back(std::make_unique<ScopedPagePauser>());
}
}

WebScopedPagePauser::~WebScopedPagePauser() {
DCHECK_NE(PagePauserStack().size(), 0u);
PagePauserStack().pop_back();
if (base::FeatureList::IsEnabled(
features::kPausePagesPerBrowsingContextGroup)) {
CHECK_NE(BrowsingContextGroupPauserStack().size(), 0u);
BrowsingContextGroupPauserStack().pop_back();
} else {
CHECK_NE(PagePauserStack().size(), 0u);
PagePauserStack().pop_back();
}
}

} // namespace blink
69 changes: 68 additions & 1 deletion third_party/blink/renderer/core/exported/web_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <string>

#include "base/functional/callback_helpers.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_mock_time_task_runner.h"
#include "base/time/time.h"
#include "build/build_config.h"
Expand All @@ -48,6 +49,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/input/web_coalesced_input_event.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/common/input/web_keyboard_event.h"
Expand Down Expand Up @@ -127,6 +129,7 @@
#include "third_party/blink/renderer/core/page/page_hidden_state.h"
#include "third_party/blink/renderer/core/page/page_popup_client.h"
#include "third_party/blink/renderer/core/page/print_context.h"
#include "third_party/blink/renderer/core/page/scoped_browsing_context_group_pauser.h"
#include "third_party/blink/renderer/core/page/scoped_page_pauser.h"
#include "third_party/blink/renderer/core/paint/paint_layer.h"
#include "third_party/blink/renderer/core/paint/paint_layer_painter.h"
Expand Down Expand Up @@ -5261,6 +5264,10 @@ TEST_F(WebViewTest, PasswordFieldEditingIsUserGesture) {
// Verify that a WebView created with a ScopedPagePauser already on the
// stack defers its loads.
TEST_F(WebViewTest, CreatedDuringPagePause) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(
features::kPausePagesPerBrowsingContextGroup);

{
WebViewImpl* web_view = web_view_helper_.Initialize();
EXPECT_FALSE(web_view->GetPage()->Paused());
Expand All @@ -5273,6 +5280,37 @@ TEST_F(WebViewTest, CreatedDuringPagePause) {
}
}

// Similar to CreatedDuringPagePause, but pauses only pages that belong to the
// same browsing context group.
TEST_F(WebViewTest, CreatedDuringBrowsingContextGroupPause) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
features::kPausePagesPerBrowsingContextGroup);

WebViewImpl* opener_webview = web_view_helper_.Initialize();
EXPECT_FALSE(opener_webview->GetPage()->Paused());

auto pauser = std::make_unique<ScopedBrowsingContextGroupPauser>(
*opener_webview->GetPage());
EXPECT_TRUE(opener_webview->GetPage()->Paused());

frame_test_helpers::WebViewHelper web_view_helper2;
WebViewImpl* webview2 =
web_view_helper2.InitializeWithOpener(opener_webview->MainFrame());
EXPECT_TRUE(webview2->GetPage()->Paused());

// The following page does not belong to the same browsing context group so
// it should not be paused.
frame_test_helpers::WebViewHelper web_view_helper3;
WebViewImpl* webview3 = web_view_helper3.Initialize();
EXPECT_FALSE(webview3->GetPage()->Paused());

// Removing the pauser should unpause pages.
pauser.reset();
EXPECT_FALSE(opener_webview->GetPage()->Paused());
EXPECT_FALSE(webview2->GetPage()->Paused());
}

// Make sure the SubframeBeforeUnloadUseCounter is only incremented on subframe
// unloads. crbug.com/635029.
TEST_F(WebViewTest, SubframeBeforeUnloadUseCounter) {
Expand Down Expand Up @@ -5317,9 +5355,13 @@ TEST_F(WebViewTest, SubframeBeforeUnloadUseCounter) {
}
}

// Verify that page loads are deferred until all ScopedPageLoadDeferrers are
// Verify that page loads are deferred until all ScopedPagePausers are
// destroyed.
TEST_F(WebViewTest, NestedPagePauses) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(
features::kPausePagesPerBrowsingContextGroup);

WebViewImpl* web_view = web_view_helper_.Initialize();
EXPECT_FALSE(web_view->GetPage()->Paused());

Expand All @@ -5338,6 +5380,31 @@ TEST_F(WebViewTest, NestedPagePauses) {
EXPECT_FALSE(web_view->GetPage()->Paused());
}

// Similar to NestedPagePauses but uses ScopedBrowsingContextGroupPauser
// instead.
TEST_F(WebViewTest, NestedPagePausesPerBrowsingContextGroup) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
features::kPausePagesPerBrowsingContextGroup);

WebViewImpl* web_view = web_view_helper_.Initialize();
EXPECT_FALSE(web_view->GetPage()->Paused());

{
ScopedBrowsingContextGroupPauser pauser(*web_view->GetPage());
EXPECT_TRUE(web_view->GetPage()->Paused());

{
ScopedBrowsingContextGroupPauser pauser2(*web_view->GetPage());
EXPECT_TRUE(web_view->GetPage()->Paused());
}

EXPECT_TRUE(web_view->GetPage()->Paused());
}

EXPECT_FALSE(web_view->GetPage()->Paused());
}

TEST_F(WebViewTest, ClosingPageIsPaused) {
WebViewImpl* web_view = web_view_helper_.Initialize();
Page* page = web_view_helper_.GetWebView()->GetPage();
Expand Down
8 changes: 7 additions & 1 deletion third_party/blink/renderer/core/frame/frame_test_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,12 @@ void WebViewHelper::InitializeWebView(
class WebView* opener,
absl::optional<blink::FencedFrame::DeprecatedFencedFrameMode>
fenced_frame_mode) {
auto browsing_context_group_info = BrowsingContextGroupInfo::CreateUnique();
if (opener) {
WebViewImpl* opener_impl = To<WebViewImpl>(opener);
browsing_context_group_info.browsing_context_group_token =
opener_impl->GetPage()->BrowsingContextGroupToken();
}
web_view_client =
CreateDefaultClientIfNeeded(web_view_client, owned_web_view_client_);
web_view_ = To<WebViewImpl>(
Expand All @@ -726,7 +732,7 @@ void WebViewHelper::InitializeWebView(
*agent_group_scheduler_,
/*session_storage_namespace_id=*/base::EmptyString(),
/*page_base_background_color=*/absl::nullopt,
BrowsingContextGroupInfo::CreateUnique()));
std::move(browsing_context_group_info)));
// This property must be set at initialization time, it is not supported to be
// changed afterward, and does nothing.
web_view_->GetSettings()->SetViewportEnabled(viewport_enabled_);
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/page/build.gni
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ blink_core_sources_page = [
"popup_opening_observer.h",
"print_context.cc",
"print_context.h",
"scoped_browsing_context_group_pauser.cc",
"scoped_browsing_context_group_pauser.h",
"scoped_page_pauser.cc",
"scoped_page_pauser.h",
"scrolling/element_fragment_anchor.cc",
Expand Down
13 changes: 12 additions & 1 deletion third_party/blink/renderer/core/page/page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include "third_party/blink/renderer/core/page/plugin_data.h"
#include "third_party/blink/renderer/core/page/plugins_changed_observer.h"
#include "third_party/blink/renderer/core/page/pointer_lock_controller.h"
#include "third_party/blink/renderer/core/page/scoped_browsing_context_group_pauser.h"
#include "third_party/blink/renderer/core/page/scoped_page_pauser.h"
#include "third_party/blink/renderer/core/page/scrolling/overscroll_controller.h"
#include "third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h"
Expand Down Expand Up @@ -171,8 +172,18 @@ Page* Page::CreateOrdinary(
}

OrdinaryPages().insert(page);
if (ScopedPagePauser::IsActive())

bool should_pause = false;
if (base::FeatureList::IsEnabled(
features::kPausePagesPerBrowsingContextGroup)) {
should_pause = ScopedBrowsingContextGroupPauser::IsActive(*page);
} else {
should_pause = ScopedPagePauser::IsActive();
}
if (should_pause) {
page->SetPaused(true);
}

return page;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// 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 "third_party/blink/renderer/core/page/scoped_browsing_context_group_pauser.h"

#include <limits>
#include <map>

#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"

namespace blink {

namespace {

uint64_t& PausedCountPerBrowsingContextGroup(
const base::UnguessableToken& token) {
using BrowsingContextGroupMap = std::map<base::UnguessableToken, uint64_t>;
DEFINE_STATIC_LOCAL(BrowsingContextGroupMap, counts, ());
return counts[token];
}

} // namespace

// static
bool ScopedBrowsingContextGroupPauser::IsActive(Page& page) {
return PausedCountPerBrowsingContextGroup(page.BrowsingContextGroupToken()) >
0;
}

ScopedBrowsingContextGroupPauser::ScopedBrowsingContextGroupPauser(Page& page)
: browsing_context_group_token_(page.BrowsingContextGroupToken()) {
CHECK_LT(PausedCount(), std::numeric_limits<uint64_t>::max());
if (++PausedCount() > 1) {
return;
}

SetPaused(true);
}

ScopedBrowsingContextGroupPauser::~ScopedBrowsingContextGroupPauser() {
CHECK_GE(PausedCount(), 1u);
if (--PausedCount() > 0) {
return;
}

SetPaused(false);
}

uint64_t& ScopedBrowsingContextGroupPauser::PausedCount() {
return PausedCountPerBrowsingContextGroup(browsing_context_group_token_);
}

void ScopedBrowsingContextGroupPauser::SetPaused(bool paused) {
HeapVector<Member<Page>> pages(Page::OrdinaryPages());
for (const auto& page : pages) {
if (page->BrowsingContextGroupToken() == browsing_context_group_token_) {
page->SetPaused(paused);
}
}
}

} // namespace blink

0 comments on commit 05e9df2

Please sign in to comment.