Skip to content

Commit

Permalink
[beacon-api] Sends all beacons (of a document/RFH) on user navigates …
Browse files Browse the repository at this point in the history 
…away to a different document.

This is to mitigate potential privacy issue that when network changes
after users think they have left a page, beacons queued in that page
still exist and get sent through the new network, which leaks navigation history to the new network. See [1] for more details.

This CL adds the following changes:

1) In browser: Add a call to `PendingBeaconHost::SendAllOnPagehide()` from `RenderFrameHostManager::UnloadOldFrame()` to let old RFH send out all its pending beacons before it is put into BFCache or being unloaded.

2) In renderer: Update `PendingBeaconDispatcher` to be called on to update all its pending beacons' states before a `pagehide` event is dispatched to event listeners.

This change makes `backgroundTimeout` property useless in the scenario that users navigate away to a different page. But it can still work in other cases like when tab is minimized or when switching tabs.

[1]: WICG/pending-beacon#30

Bug: 1293679
Change-Id: I5a7ddf4284717e1af482286dd6f56344d4ef4b26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3881967
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ming-Ying Chung <mych@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1047323}
  • Loading branch information
@mingyc
mingyc authored and Chromium LUCI CQ committed Sep 15, 2022
1 parent 9690848 commit 2be095e
Show file tree
Hide file tree
Showing 18 changed files with 868 additions and 218 deletions.
256 changes: 191 additions & 65 deletions content/browser/renderer_host/pending_beacon_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ MATCHER(IsFrameHidden,

class PendingBeaconTimeoutBrowserTestBase : public ContentBrowserTest {
protected:
using FeaturesType =
std::vector<base::test::ScopedFeatureList::FeatureAndParams>;

void SetUp() override {
feature_list_.InitWithFeaturesAndParameters(GetEnabledFeatures(), {});
ContentBrowserTest::SetUp();
}
virtual const FeaturesType& GetEnabledFeatures() = 0;

void SetUpOnMainThread() override {
CheckPermissionStatus(blink::PermissionType::BACKGROUND_SYNC,
blink::mojom::PermissionStatus::GRANTED);
Expand Down Expand Up @@ -190,6 +199,8 @@ class PendingBeaconTimeoutBrowserTestBase : public ContentBrowserTest {
return web_contents()->GetPrimaryMainFrame();
}

base::test::ScopedFeatureList feature_list_;

base::Lock count_lock_;
size_t sent_beacon_count_ GUARDED_BY(count_lock_) = 0;
std::unique_ptr<base::RunLoop> waiting_run_loop_;
Expand All @@ -198,6 +209,37 @@ class PendingBeaconTimeoutBrowserTestBase : public ContentBrowserTest {
std::unique_ptr<RenderFrameHostWrapper> previous_document_ = nullptr;
};

class PendingBeaconWithBackForwardCacheMetricsBrowserTestBase
: public PendingBeaconTimeoutBrowserTestBase,
public BackForwardCacheMetricsTestMatcher {
protected:
void SetUpOnMainThread() override {
// TestAutoSetUkmRecorder's constructor requires a sequenced context.
ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
histogram_tester_ = std::make_unique<base::HistogramTester>();
PendingBeaconTimeoutBrowserTestBase::SetUpOnMainThread();
}

void TearDownOnMainThread() override {
ukm_recorder_.reset();
histogram_tester_.reset();
PendingBeaconTimeoutBrowserTestBase::TearDownOnMainThread();
}

// `BackForwardCacheMetricsTestMatcher` implementation.
const ukm::TestAutoSetUkmRecorder& ukm_recorder() override {
return *ukm_recorder_;
}
// `BackForwardCacheMetricsTestMatcher` implementation.
const base::HistogramTester& histogram_tester() override {
return *histogram_tester_;
}

private:
std::unique_ptr<ukm::TestAutoSetUkmRecorder> ukm_recorder_;
std::unique_ptr<base::HistogramTester> histogram_tester_;
};

struct TestTimeoutType {
std::string test_case_name;
int32_t timeout;
Expand All @@ -213,16 +255,12 @@ class PendingBeaconTimeoutNoBackForwardCacheBrowserTest
: public PendingBeaconTimeoutBrowserTestBase,
public testing::WithParamInterface<TestTimeoutType> {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
feature_list_.InitWithFeaturesAndParameters(
{{blink::features::kPendingBeaconAPI, {}},
{features::kBackForwardCache, {{"cache_size", "0"}}}},
{});
PendingBeaconTimeoutBrowserTestBase::SetUpCommandLine(command_line);
const FeaturesType& GetEnabledFeatures() override {
static const FeaturesType enabled_features = {
{blink::features::kPendingBeaconAPI, {{"send_on_navigation", "true"}}},
{features::kBackForwardCache, {{"cache_size", "0"}}}};
return enabled_features;
}

private:
base::test::ScopedFeatureList feature_list_;
};

INSTANTIATE_TEST_SUITE_P(
Expand Down Expand Up @@ -253,7 +291,7 @@ IN_PROC_BROWSER_TEST_P(PendingBeaconTimeoutNoBackForwardCacheBrowserTest,
kBeaconEndpoint, GetParam().timeout));
ASSERT_TRUE(WaitUntilPreviousDocumentDeleted());

WaitForAllBeaconsSent(total_beacon);
// The beacon should have been sent out after the page is gone.
EXPECT_EQ(sent_beacon_count(), total_beacon);
}

Expand All @@ -270,7 +308,7 @@ IN_PROC_BROWSER_TEST_P(PendingBeaconTimeoutNoBackForwardCacheBrowserTest,
kBeaconEndpoint, GetParam().timeout));
ASSERT_TRUE(WaitUntilPreviousDocumentDeleted());

WaitForAllBeaconsSent(total_beacon);
// The beacon should have been sent out after the page is gone.
EXPECT_EQ(sent_beacon_count(), total_beacon);
}

Expand All @@ -282,21 +320,19 @@ IN_PROC_BROWSER_TEST_P(PendingBeaconTimeoutNoBackForwardCacheBrowserTest,
class PendingBeaconBackgroundTimeoutBrowserTest
: public PendingBeaconTimeoutBrowserTestBase {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
feature_list_.InitWithFeaturesAndParameters(
{{blink::features::kPendingBeaconAPI,
{{"PendingBeaconMaxBackgroundTimeoutInMs", "10000"}}},
{features::kBackForwardCache,
{{"TimeToLiveInBackForwardCacheInSeconds", "5"}}},
// Forces BFCache to work in low memory device.
{features::kBackForwardCacheMemoryControls,
{{"memory_threshold_for_back_forward_cache_in_mb", "0"}}}},
{});
PendingBeaconTimeoutBrowserTestBase::SetUpCommandLine(command_line);
const FeaturesType& GetEnabledFeatures() override {
static const FeaturesType enabled_features = {
{blink::features::kPendingBeaconAPI,
{{"PendingBeaconMaxBackgroundTimeoutInMs", "60000"},
// Don't force sending out beacons on pagehide.
{"send_on_navigation", "false"}}},
{features::kBackForwardCache,
{{"TimeToLiveInBackForwardCacheInSeconds", "5"}}},
// Forces BFCache to work in low memory device.
{features::kBackForwardCacheMemoryControls,
{{"memory_threshold_for_back_forward_cache_in_mb", "0"}}}};
return enabled_features;
}

private:
base::test::ScopedFeatureList feature_list_;
};

IN_PROC_BROWSER_TEST_F(PendingBeaconBackgroundTimeoutBrowserTest,
Expand Down Expand Up @@ -518,48 +554,20 @@ IN_PROC_BROWSER_TEST_F(PendingBeaconTimeoutBrowserTest, SendMultipleOnTimeout) {
// Sets a long BFCache timeout (1min) so that beacon won't be sent out due to
// page eviction.
class PendingBeaconMutualTimeoutWithLongBackForwardCacheTTLBrowserTest
: public PendingBeaconTimeoutBrowserTestBase,
public BackForwardCacheMetricsTestMatcher {
: public PendingBeaconWithBackForwardCacheMetricsBrowserTestBase {
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
feature_list_.InitWithFeaturesAndParameters(
{{blink::features::kPendingBeaconAPI, {}},
{features::kBackForwardCache,
{{"TimeToLiveInBackForwardCacheInSeconds", "60"}}},
// Forces BFCache to work in low memory device.
{features::kBackForwardCacheMemoryControls,
{{"memory_threshold_for_back_forward_cache_in_mb", "0"}}}},
{});
PendingBeaconTimeoutBrowserTestBase::SetUpCommandLine(command_line);
}

void SetUpOnMainThread() override {
// TestAutoSetUkmRecorder's constructor requires a sequenced context.
ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
histogram_tester_ = std::make_unique<base::HistogramTester>();
PendingBeaconTimeoutBrowserTestBase::SetUpOnMainThread();
}

void TearDownOnMainThread() override {
ukm_recorder_.reset();
histogram_tester_.reset();
PendingBeaconTimeoutBrowserTestBase::TearDownOnMainThread();
}

// `BackForwardCacheMetricsTestMatcher` implementation.
const ukm::TestAutoSetUkmRecorder& ukm_recorder() override {
return *ukm_recorder_;
}
// `BackForwardCacheMetricsTestMatcher` implementation.
const base::HistogramTester& histogram_tester() override {
return *histogram_tester_;
const FeaturesType& GetEnabledFeatures() override {
static const FeaturesType enabled_features = {
{blink::features::kPendingBeaconAPI,
{// Don't force sending out beacons on pagehide.
{"send_on_navigation", "false"}}},
{features::kBackForwardCache,
{{"TimeToLiveInBackForwardCacheInSeconds", "60"}}},
// Forces BFCache to work in low memory device.
{features::kBackForwardCacheMemoryControls,
{{"memory_threshold_for_back_forward_cache_in_mb", "0"}}}};
return enabled_features;
}

private:
base::test::ScopedFeatureList feature_list_;

std::unique_ptr<ukm::TestAutoSetUkmRecorder> ukm_recorder_;
std::unique_ptr<base::HistogramTester> histogram_tester_;
};

IN_PROC_BROWSER_TEST_F(
Expand Down Expand Up @@ -631,4 +639,122 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(sent_beacon_count(), total_beacon);
}

// Tests to cover PendingBeacon's behaviors when enabled forced sending on
// pagehide event.
//
// Setting a long `PendingBeaconMaxBackgroundTimeoutInMs` (1min), and a long
// BFCache timeout (1min) so that beacon sending cannot be caused by reaching
// max background timeout limit, and cannot be caused by BFCache eviction.
class PendingBeaconSendOnPagehideBrowserTest
: public PendingBeaconWithBackForwardCacheMetricsBrowserTestBase {
protected:
const FeaturesType& GetEnabledFeatures() override {
static const FeaturesType enabled_features = {
{blink::features::kPendingBeaconAPI,
{{"PendingBeaconMaxBackgroundTimeoutInMs", "60000"},
{"send_on_navigation", "true"}}},
{features::kBackForwardCache,
{{"TimeToLiveInBackForwardCacheInSeconds", "60"}}},
// Forces BFCache to work in low memory device.
{features::kBackForwardCacheMemoryControls,
{{"memory_threshold_for_back_forward_cache_in_mb", "0"}}}};
return enabled_features;
}
};

IN_PROC_BROWSER_TEST_F(PendingBeaconSendOnPagehideBrowserTest,
SendOnPagehideWhenPageIsPersisted) {
const size_t total_beacon = 3;
RegisterBeaconRequestMonitor(total_beacon);

// Creates 3 pending beacons with default backgroundTimeout & timeout.
// They should be sent out on transitioning to pagehide event.
RunScriptInANavigateToB(JsReplace(R"(
document.title = '';
let p1 = new PendingGetBeacon($1);
let p2 = new PendingPostBeacon($1);
let p3 = new PendingGetBeacon($1);
window.addEventListener('pagehide', (e) => {
document.title = e.persisted + '/' + p1.pending + '/' + p2.pending +
'/' + p3.pending;
});
)",
kBeaconEndpoint));
ASSERT_THAT(previous_document(), IsFrameHidden());

// Navigate back to A.
ASSERT_TRUE(HistoryGoBack(web_contents()));
// The same page A is still alive.
ExpectRestored(FROM_HERE);
// All beacons should have been sent out before previous pagehide.
std::u16string expected_title = u"true/false/false/false";
TitleWatcher title_watcher(web_contents(), expected_title);
EXPECT_EQ(title_watcher.WaitAndGetTitle(), expected_title);
EXPECT_EQ(sent_beacon_count(), total_beacon);
}

IN_PROC_BROWSER_TEST_F(PendingBeaconSendOnPagehideBrowserTest,
SendOnPagehideBeforeBackgroundTimeout) {
const size_t total_beacon = 3;
RegisterBeaconRequestMonitor(total_beacon);

// Creates 3 pending beacons with long backgroundTimeout < BFCache TTL (1min).
// They should be sent out on transitioning to pagehide but before the end of
// backgroundTimeout and before BFCache TTL.
RunScriptInANavigateToB(JsReplace(R"(
document.title = '';
let p1 = new PendingGetBeacon($1, {backgroundTimeout: 20000});
let p2 = new PendingPostBeacon($1, {backgroundTimeout: 15000});
let p3 = new PendingGetBeacon($1, {backgroundTimeout: 10000});
window.addEventListener('pagehide', (e) => {
document.title = e.persisted + '/' + p1.pending + '/' + p2.pending +
'/' + p3.pending;
});
)",
kBeaconEndpoint));
ASSERT_THAT(previous_document(), IsFrameHidden());

// Navigate back to A.
ASSERT_TRUE(HistoryGoBack(web_contents()));
// The same page A is still alive.
ExpectRestored(FROM_HERE);
// All beacons should have been sent out.
std::u16string expected_title = u"true/false/false/false";
TitleWatcher title_watcher(web_contents(), expected_title);
EXPECT_EQ(title_watcher.WaitAndGetTitle(), expected_title);
EXPECT_EQ(sent_beacon_count(), total_beacon);
}

IN_PROC_BROWSER_TEST_F(PendingBeaconSendOnPagehideBrowserTest,
SendOnPagehideBeforeTimeout) {
const size_t total_beacon = 3;
RegisterBeaconRequestMonitor(total_beacon);

// Creates 3 pending beacons with long timeout < BFCache TTL (1min).
// They should be sent out on transitioning to pagehide but before the end of
// timeout and before BFCache TTL.
RunScriptInANavigateToB(JsReplace(R"(
document.title = '';
let p1 = new PendingGetBeacon($1, {timeout: 20000});
let p2 = new PendingPostBeacon($1, {timeout: 10000});
let p3 = new PendingGetBeacon($1, {timeout: 15000});
window.addEventListener('pagehide', (e) => {
document.title = e.persisted + '/' + p1.pending + '/' + p2.pending +
'/' + p3.pending;
});
)",
kBeaconEndpoint));
ASSERT_THAT(previous_document(), IsFrameHidden());

// Navigate back to A.
ASSERT_TRUE(HistoryGoBack(web_contents()));
// The same page A is still alive.
ExpectRestored(FROM_HERE);
// All beacons should have been sent out.
std::u16string expected_title = u"true/false/false/false";
TitleWatcher title_watcher(web_contents(), expected_title);
EXPECT_EQ(title_watcher.WaitAndGetTitle(), expected_title);
EXPECT_EQ(sent_beacon_count(), total_beacon);
}

} // namespace content
24 changes: 24 additions & 0 deletions content/browser/renderer_host/pending_beacon_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/mojom/fetch_api.mojom.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/permissions/permission_utils.h"

namespace content {
Expand Down Expand Up @@ -104,6 +105,7 @@ void PendingBeaconHost::Send(
if (beacons.empty()) {
return;
}

service_->SendBeacons(beacons, shared_url_factory_.get());
}

Expand All @@ -112,6 +114,28 @@ void PendingBeaconHost::SetReceiver(
receiver_.Bind(std::move(receiver));
}

void PendingBeaconHost::SendAllOnNavigation() {
if (!blink::features::kPendingBeaconAPIForcesSendingOnNavigation.Get()) {
return;
}

// Sends out all `beacons_` ASAP to avoid network change happens.
// This is to mitigate potential privacy issue that when network changes
// after users think they have left a page, beacons queued in that page
// still exist and get sent through the new network, which leaks navigation
// history to the new network.
// See https://github.com/WICG/unload-beacon/issues/30.

// Swaps out from private field first to make any potential subsequent send
// requests from renderer no-ops.
std::vector<std::unique_ptr<Beacon>> to_send;
to_send.swap(beacons_);
Send(to_send);

// Now all beacons are gone.
// The renderer-side beacons should update their pending states by themselves.
}

DOCUMENT_USER_DATA_KEY_IMPL(PendingBeaconHost);

void Beacon::Deactivate() {
Expand Down

0 comments on commit 2be095e

Please sign in to comment.