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}
NOKEYCHECK=True
GitOrigin-RevId: 2be095e89fa5007ea5bc1bfe4419165471be3454
  • Loading branch information
mingyc authored and Copybara-Service committed Sep 15, 2022
1 parent 1131806 commit 30d09d1
Show file tree
Hide file tree
Showing 11 changed files with 492 additions and 56 deletions.
2 changes: 2 additions & 0 deletions blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1356,6 +1356,8 @@ const base::Feature kPendingBeaconAPI{"PendingBeaconAPI",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::FeatureParam<bool> kPendingBeaconAPIRequiresOriginTrial = {
&kPendingBeaconAPI, "requires_origin_trial", false};
const base::FeatureParam<bool> kPendingBeaconAPIForcesSendingOnNavigation = {
&blink::features::kPendingBeaconAPI, "send_on_navigation", true};

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
const base::Feature kPrefetchFontLookupTables{
Expand Down
5 changes: 5 additions & 0 deletions blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,11 @@ BLINK_COMMON_EXPORT extern const base::Feature kPendingBeaconAPI;
// both in Chromium & in Blink.
BLINK_COMMON_EXPORT extern const base::FeatureParam<bool>
kPendingBeaconAPIRequiresOriginTrial;
// Allows control to decide whether to forced sending out beacons on navigating
// away a page (transitioning to dispatch pagehide event).
// Details in https://github.com/WICG/unload-beacon/issues/30
BLINK_COMMON_EXPORT extern const base::FeatureParam<bool>
kPendingBeaconAPIForcesSendingOnNavigation;

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_ANDROID)
// If enabled, font lookup tables will be prefetched on renderer startup.
Expand Down
13 changes: 12 additions & 1 deletion blink/public/mojom/frame/pending_beacon.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ enum BeaconMethod {
};

// Interface for creating browser-side pending beacon objects.
//
// There is one instance of this interface per RenderFrameHost in the browser
// process.
//
// All methods are called by renderer.
//
// API explainer here:
// https://github.com/WICG/unload-beacon/blob/main/README.md
interface PendingBeaconHost {
Expand All @@ -30,7 +36,12 @@ interface PendingBeaconHost {

};

// Interface for configuring and acting on pending beacons.
// Interface for configuring and acting on a pending beacon in the browser.
//
// A pending beacon in the renderer process uses this interface to communicate
// with its counterpart in the browser process.
//
// All methods are called by renderer.
interface PendingBeacon {

// Deactivates the pending beacon. After this call it will not be sent.
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/frame/build.gni
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ blink_core_tests_frame = [
"mhtml_loading_test.cc",
"performance_monitor_test.cc",
"pending_beacon_dispatcher_test.cc",
"pending_beacon_test.cc",
"policy_container_test.cc",
"report_test.cc",
"reporting_context_test.cc",
Expand Down
9 changes: 9 additions & 0 deletions blink/renderer/core/frame/local_dom_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
#include "third_party/blink/renderer/core/frame/local_frame_client.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/frame/navigator.h"
#include "third_party/blink/renderer/core/frame/pending_beacon_dispatcher.h"
#include "third_party/blink/renderer/core/frame/permissions_policy_violation_report_body.h"
#include "third_party/blink/renderer/core/frame/report.h"
#include "third_party/blink/renderer/core/frame/reporting_context.h"
Expand Down Expand Up @@ -836,6 +837,14 @@ void LocalDOMWindow::DispatchPagehideEvent(
// TODO(crbug.com/1119291): Investigate whether this is possible or not.
return;
}

if (base::FeatureList::IsEnabled(features::kPendingBeaconAPI)) {
if (auto* dispatcher =
PendingBeaconDispatcher::From(*GetExecutionContext())) {
dispatcher->OnDispatchPagehide();
}
}

DispatchEvent(
*PageTransitionEvent::Create(event_type_names::kPagehide, persistence),
document_.Get());
Expand Down
8 changes: 2 additions & 6 deletions blink/renderer/core/frame/pending_beacon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ void PendingBeacon::deactivate() {
remote_->Deactivate();
pending_ = false;

auto* dispatcher = PendingBeaconDispatcher::From(*ec_);
DCHECK(dispatcher);
dispatcher->Unregister(this);
UnregisterFromDispatcher();
}
}

Expand All @@ -97,9 +95,7 @@ void PendingBeacon::sendNow() {
remote_->SendNow();
pending_ = false;

auto* dispatcher = PendingBeaconDispatcher::From(*ec_);
DCHECK(dispatcher);
dispatcher->Unregister(this);
UnregisterFromDispatcher();
}
}

Expand Down
6 changes: 6 additions & 0 deletions blink/renderer/core/frame/pending_beacon.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class CORE_EXPORT PendingBeacon
// `PendingBeaconDispatcher::PendingBeacon` implementation.
base::TimeDelta GetBackgroundTimeout() const override;
void Send() override;
bool IsPending() const override { return pending_; }
void MarkNotPending() override { pending_ = false; }
ExecutionContext* GetExecutionContext() override { return ec_; }

protected:
explicit PendingBeacon(ExecutionContext* context,
Expand All @@ -74,10 +77,13 @@ class CORE_EXPORT PendingBeacon
// A convenient method to return a TaskRunner which is able to keep working
// even if the JS context is frozen.
scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner();
// Triggered by `timeout_timer_`.
void TimeoutTimerFired(TimerBase*);

Member<ExecutionContext> ec_;
// Connects to a PendingBeacon in the browser process.
HeapMojoRemote<mojom::blink::PendingBeacon> remote_;

String url_;
const String method_;
base::TimeDelta background_timeout_;
Expand Down
39 changes: 39 additions & 0 deletions blink/renderer/core/frame/pending_beacon_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ struct ReverseBeaconTimeoutSorter {

} // namespace

void PendingBeaconDispatcher::PendingBeacon::UnregisterFromDispatcher() {
auto* ec = GetExecutionContext();
DCHECK(ec);
auto* dispatcher = PendingBeaconDispatcher::From(*ec);
DCHECK(dispatcher);
dispatcher->Unregister(this);
}

// static
const char PendingBeaconDispatcher::kSupplementName[] =
"PendingBeaconDispatcher";
Expand Down Expand Up @@ -278,4 +286,35 @@ void PendingBeaconDispatcher::Trace(Visitor* visitor) const {
visitor->Trace(background_timeout_descending_beacons_);
}

bool PendingBeaconDispatcher::HasPendingBeaconForTesting(
PendingBeacon* pending_beacon) const {
return pending_beacons_.Contains(pending_beacon);
}

void PendingBeaconDispatcher::OnDispatchPagehide() {
if (!features::kPendingBeaconAPIForcesSendingOnNavigation.Get()) {
return;
}

// At this point, the renderer can assume that all beacons on this document
// have (or will have) been sent out by browsers. The only work left is to
// update all beacons pending state such that they cannot be updated anymore.
//
// 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.
//
// Note that the pagehide event might be dispatched a bit earlier than when
// beacons get sents by browser in same-site navigation.

for (auto& pending_beacon : pending_beacons_) {
if (pending_beacon->IsPending()) {
pending_beacon->MarkNotPending();
}
}
pending_beacons_.clear();
}

} // namespace blink
57 changes: 54 additions & 3 deletions blink/renderer/core/frame/pending_beacon_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_PENDING_BEACON_DISPATCHER_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_FRAME_PENDING_BEACON_DISPATCHER_H_

#include "base/gtest_prod_util.h"
#include "base/time/time.h"
#include "base/types/pass_key.h"
#include "third_party/blink/public/mojom/frame/pending_beacon.mojom-blink.h"
Expand Down Expand Up @@ -79,10 +80,32 @@ class CORE_EXPORT PendingBeaconDispatcher
// Implementation should ensure the returned TimeDelta is not negative.
virtual base::TimeDelta GetBackgroundTimeout() const = 0;
// Triggers beacon sending action.
// Implementation should also transitions this beacon into non-pending
// state. and call `PendingBeaconDispatcher::Unregister()` to unregister
// itself from further scheduling.
//
// The sending action may not be triggered if it decides not to do so.
// If triggered, implementation should also transitions this beacon into
// non-pending state, and call `PendingBeaconDispatcher::Unregister()` to
// unregister itself from further scheduling.
// If not triggered, the dispatcher will schedule to send this next time as
// long as this is still registered.
virtual void Send() = 0;

virtual bool IsPending() const = 0;
virtual void MarkNotPending() = 0;
// Provides ExecutionContext where this beacon is created.
virtual ExecutionContext* GetExecutionContext() = 0;

protected:
// Unregisters this beacon from the PendingBeaconDispatcher associated with
// `GetExecutionContext()`.
//
// Calling this method will reduce the lifetime of this instance back to the
// lifetime of the corresponding JS object, i.e. it won't be extended by the
// PendingBeaconDispatcher anymore.
//
// After this call, all existing timers, either in this PendingBeacon or in
// PendingBeaconDispatcher, are not cancelled, but will be no-op when their
// callbacks are triggered.
void UnregisterFromDispatcher();
};

static const char kSupplementName[];
Expand Down Expand Up @@ -139,6 +162,16 @@ class CORE_EXPORT PendingBeaconDispatcher
// `PageVisibilityObserver` implementation.
void PageVisibilityChanged() override;

// Handles pagehide event.
//
// The browser will force sending out all beacons on navigating to a new page,
// i.e. on pagehide event. Whether or not the old page is put into
// BackForwardCache is not important.
//
// This method asks all owned `pending_beacons_` to update their state to
// non-pending and unregisters them from this dispatcher.
void OnDispatchPagehide();

private:
// Schedules a series of tasks to dispatch pending beacons according to
// their `PendingBeacon::GetBackgroundTimeout()`.
Expand Down Expand Up @@ -207,6 +240,24 @@ class CORE_EXPORT PendingBeaconDispatcher
//
// It is canceled when `CancelDispatchBeacons()` is called.
TaskHandle task_handle_;

// For testing:
bool HasPendingBeaconForTesting(PendingBeacon* pending_beacon) const;
FRIEND_TEST_ALL_PREFIXES(PendingBeaconDispatcherBasicBeaconsTest,
DispatchBeaconsOnBackgroundTimeout);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconDispatcherBackgroundTimeoutBundledTest,
DispatchOrderedBeacons);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconDispatcherBackgroundTimeoutBundledTest,
DispatchReversedBeacons);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconDispatcherBackgroundTimeoutBundledTest,
DispatchDuplicatedBeacons);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconDispatcherOnPagehideTest,
OnPagehideUpdateAndUnregisterAllBeacons);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconCreateTest, Create);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconSendTest, Send);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconSendTest, SendNow);
FRIEND_TEST_ALL_PREFIXES(PendingBeaconSendTest,
SetNonPendingAfterTimeoutTimerStart);
};

} // namespace blink
Expand Down
Loading

0 comments on commit 30d09d1

Please sign in to comment.