Skip to content

Commit

Permalink
[beacon-api] Update PendingBeacon IDL to match latest explainer
Browse files Browse the repository at this point in the history
- `pageHideTimeout`
  - Replace pageHideTimeout with `backgroundTimeout` and `timeout`.
  - Allow to set them in constructor & via property setters.
- `isPending`
  - Rename isPending to `pending` and specify its behaviors.
- `setData(data)`
  - Remove setData(data) from base class PendingBeacon.
  - Add subclass `PendingGetBeacon` which supports `setURL(url)`.
    - Also define a new mojom method `setRequestURL(url)` to help update
      browser-side beacon.
  - Add subclass `PendingPostBeacon` which supports `setData(data)`.

See [the pull request][1] or [explainer][2] for more details.

[1]: WICG/pending-beacon#16 (comment)
[2]: https://github.com/WICG/unload-beacon#readme

Bug: 1293679
Change-Id: I8b73bdd3de7f5e8a3358564be52e2c0f2f6774de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3789544
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Ming-Ying Chung <mych@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031758}
  • Loading branch information
mingyc authored and Chromium LUCI CQ committed Aug 5, 2022
1 parent 7456a3b commit d65b747
Show file tree
Hide file tree
Showing 26 changed files with 611 additions and 281 deletions.
9 changes: 9 additions & 0 deletions content/browser/renderer_host/pending_beacon_host.cc
Expand Up @@ -111,6 +111,15 @@ void Beacon::SetRequestData(
request_elements_ = std::move(*request_body->elements_mutable());
}

void Beacon::SetRequestURL(const GURL& url) {
// Only GET Beacon is allowed to update its URL after construction.
if (method_ != blink::mojom::BeaconMethod::kGet) {
mojo::ReportBadMessage("Unexpected BeaconMethod from renderer");
return;
}
url_ = url;
}

void Beacon::SendNow() {
beacon_host_->SendBeacon(this);
}
Expand Down
7 changes: 6 additions & 1 deletion content/browser/renderer_host/pending_beacon_host.h
Expand Up @@ -111,6 +111,11 @@ class Beacon : public blink::mojom::PendingBeacon {
void SetRequestData(scoped_refptr<network::ResourceRequestBody> request_body,
const std::string& content_type) override;

// Sets request url for the pending beacon.
// The spec only allows GET beacons to update its own URL. So `BeaconMethod`
// must be kGet when calling this.
void SetRequestURL(const GURL& url) override;

// Sends the beacon immediately, and deletes it from its containing
// PendingBeaconHost.
void SendNow() override;
Expand Down Expand Up @@ -139,7 +144,7 @@ class Beacon : public blink::mojom::PendingBeacon {
// The beacon host that owns this beacon. raw_ptr is safe here as the host's
// lifetime will always be longer than the individual beacon's.
raw_ptr<PendingBeaconHost> beacon_host_;
const GURL url_;
GURL url_;
[[maybe_unused]] const blink::mojom::BeaconMethod method_;
[[maybe_unused]] const base::TimeDelta timeout_;

Expand Down
17 changes: 17 additions & 0 deletions content/browser/renderer_host/pending_beacon_host_unittest.cc
Expand Up @@ -252,4 +252,21 @@ TEST_F(BeaconTest, AttemptToSetUnsafeContentTypeAndTerminated) {
EXPECT_EQ(bad_message, "Unexpected Content-Type from renderer");
}

TEST_F(BeaconTest, AttemptToSetURLForPostBeaconAndTerminated) {
auto beacon_remote =
CreateBeaconAndPassRemote(net::HttpRequestHeaders::kPostMethod);
// Intercepts Mojo bad-message error.
std::string bad_message;
mojo::SetDefaultProcessErrorHandler(
base::BindLambdaForTesting([&](const std::string& error) {
ASSERT_TRUE(bad_message.empty());
bad_message = error;
}));

beacon_remote->SetRequestURL(GURL("/test_set_url"));
beacon_remote.FlushForTesting();

EXPECT_EQ(bad_message, "Unexpected BeaconMethod from renderer");
}

} // namespace content
5 changes: 5 additions & 0 deletions third_party/blink/public/mojom/frame/pending_beacon.mojom
Expand Up @@ -49,6 +49,11 @@ interface PendingBeacon {
SetRequestData(network.mojom.URLRequestBody request_body,
string content_type);

// Sets request url for the pending beacon.
// The spec only allows GET beacons to update its own URL. So the receiving
// pending beacon's `BeaconMethod` must be kGet.
SetRequestURL(url.mojom.Url url);

// Sends the pending beacon immediately.
// Calling this will close the message pipe for the interface as well, so no
// further calls can be made.
Expand Down
Expand Up @@ -3591,16 +3591,16 @@ enum WebFeature {
kSharedStorageAPI_Run_Method = 4270,
kViewTimelineConstructor = 4271,
kH1UserAgentFontSizeInSectionApplied = 4272,
kV8PendingBeacon_Constructor = 4273,
kOBSOLETE_kV8PendingBeacon_Constructor = 4273,
kV8PendingBeacon_Url_AttributeGetter = 4274,
kOBSOLETE_kV8PendingBeacon_Url_AttributeSetter = 4275,
kV8PendingBeacon_Method_AttributeGetter = 4276,
kOBSOLETE_kV8PendingBeacon_Method_AttributeSetter = 4277,
kV8PendingBeacon_PageHideTimeout_AttributeGetter = 4278,
kOBSOLETE_kV8PendingBeacon_PageHideTimeout_AttributeGetter = 4278,
kOBSOLETE_kV8PendingBeacon_PageHideTimeout_AttributeSetter = 4279,
kOBSOLETE_kV8PendingBeacon_State_AttributeGetter = 4280,
kV8PendingBeacon_Deactivate_Method = 4281,
kV8PendingBeacon_SetData_Method = 4282,
kOBSOLETE_kV8PendingBeacon_SetData_Method = 4282,
kV8PendingBeacon_SendNow_Method = 4283,
kTabSharingBarSwitchToCapturer = 4284,
kTabSharingBarSwitchToCapturee = 4285,
Expand All @@ -3613,7 +3613,7 @@ enum WebFeature {
kDeviceMotionUsedWithoutPermissionRequest = 4292,
kPrivateNetworkAccessPermissionPrompt = 4293,
kPseudoBeforeAfterForDateTimeInputElement = 4294,
kV8PendingBeacon_IsPending_AttributeGetter = 4295,
kOBSOLETE_kV8PendingBeacon_IsPending_AttributeGetter = 4295,
kParentOfDisabledFormControlRespondsToMouseEvents = 4296,
kUnhandledExceptionCountInMainThread = 4297,
kUnhandledExceptionCountInWorker = 4298,
Expand All @@ -3637,6 +3637,15 @@ enum WebFeature {
kCrossOriginScrollIntoView = 4316,
kLinkRelCanonical = 4317,
kCredentialManagerIsConditionalMediationAvailable = 4318,
kV8PendingBeacon_Pending_AttributeGetter = 4319,
kV8PendingBeacon_BackgroundTimeout_AttributeGetter = 4320,
kV8PendingBeacon_BackgroundTimeout_AttributeSetter = 4321,
kV8PendingBeacon_Timeout_AttributeGetter = 4322,
kV8PendingBeacon_Timeout_AttributeSetter = 4323,
kV8PendingGetBeacon_Constructor = 4324,
kV8PendingGetBeacon_SetURL_Method = 4325,
kV8PendingPostBeacon_Constructor = 4326,
kV8PendingPostBeacon_SetData_Method = 4327,

// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
Expand Down
8 changes: 6 additions & 2 deletions third_party/blink/renderer/bindings/generated_in_core.gni
Expand Up @@ -567,8 +567,6 @@ generated_interface_sources_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_bar_prop.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_beacon_method.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_beacon_method.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_beacon_options.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_beacon_options.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_before_create_policy_event.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_before_create_policy_event.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_before_unload_event.cc",
Expand Down Expand Up @@ -1115,6 +1113,12 @@ generated_interface_sources_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_page_transition_event.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_beacon.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_beacon.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_beacon_options.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_beacon_options.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_get_beacon.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_get_beacon.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_post_beacon.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_pending_post_beacon.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_performance.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_performance.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_performance_element_timing.cc",
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/frame/build.gni
Expand Up @@ -144,6 +144,10 @@ blink_core_sources_frame = [
"page_scale_constraints_set.h",
"pending_beacon.cc",
"pending_beacon.h",
"pending_post_beacon.cc",
"pending_post_beacon.h",
"pending_get_beacon.cc",
"pending_get_beacon.h",
"pausable_script_executor.cc",
"pausable_script_executor.h",
"performance_monitor.cc",
Expand Down
122 changes: 42 additions & 80 deletions third_party/blink/renderer/core/frame/pending_beacon.cc
Expand Up @@ -4,16 +4,15 @@

#include "third_party/blink/renderer/core/frame/pending_beacon.h"

#include "base/time/time.h"
#include "third_party/blink/public/mojom/frame/pending_beacon.mojom-blink.h"
#include "third_party/blink/public/platform/task_type.h"
#include "third_party/blink/public/platform/web_url_request_util.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_beacon_options.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_union_arraybuffer_arraybufferview_blob_formdata_readablestream_urlsearchparams_usvstring.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/loader/beacon_data.h"
#include "third_party/blink/renderer/platform/exported/wrapped_resource_request.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
#include "third_party/blink/renderer/platform/network/encoded_form_data.h"
#include "third_party/blink/renderer/platform/network/http_names.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"

namespace blink {
Expand Down Expand Up @@ -57,107 +56,70 @@ class PendingBeaconHostRemote
const char PendingBeaconHostRemote::kSupplementName[] =
"PendingBeaconHostRemote";

// static
PendingBeacon* PendingBeacon::Create(ExecutionContext* ec,
const String& targetURL) {
BeaconOptions* options = BeaconOptions::Create();
return PendingBeacon::Create(ec, targetURL, options);
}
// static
PendingBeacon* PendingBeacon::Create(ExecutionContext* ec,
const String& targetURL,
BeaconOptions* options) {
PendingBeacon* beacon = MakeGarbageCollected<PendingBeacon>(
ec, targetURL, options->method(), options->pageHideTimeout());
mojom::blink::BeaconMethod method;
if (options->method() == V8BeaconMethod::Enum::kGET) {
method = mojom::blink::BeaconMethod::kGet;
PendingBeacon::PendingBeacon(ExecutionContext* ec,
const String& url,
const String& method,
int32_t background_timeout,
int32_t timeout)
: ec_(ec),
remote_(ec),
url_(url),
method_(method),
background_timeout_(base::Milliseconds(background_timeout)),
timeout_(base::Milliseconds(timeout)) {
mojom::blink::BeaconMethod host_method;
if (method == http_names::kGET) {
host_method = mojom::blink::BeaconMethod::kGet;
} else {
method = mojom::blink::BeaconMethod::kPost;
host_method = mojom::blink::BeaconMethod::kPost;
}

// Using the MiscPlatformAPI task type as pending beacons are not yet
// associated with any specific task runner in the spec.
auto task_runner = ec->GetTaskRunner(TaskType::kMiscPlatformAPI);

mojo::PendingReceiver<mojom::blink::PendingBeacon> beacon_receiver =
beacon->remote_.BindNewPipeAndPassReceiver(task_runner);
remote_.BindNewPipeAndPassReceiver(task_runner);
KURL host_url = ec->CompleteURL(url);

PendingBeaconHostRemote& host_remote = PendingBeaconHostRemote::From(*ec);

KURL url = ec->CompleteURL(targetURL);

host_remote.remote_->CreateBeacon(
std::move(beacon_receiver), url, method,
base::Milliseconds(beacon->page_hide_timeout_));
return beacon;
host_remote.remote_->CreateBeacon(std::move(beacon_receiver), host_url,
host_method, background_timeout_);
}

PendingBeacon::PendingBeacon(ExecutionContext* context,
String url,
String method,
int32_t page_hide_timeout)
: remote_(context),
url_(url),
method_(method),
page_hide_timeout_(page_hide_timeout) {}

void PendingBeacon::Trace(Visitor* visitor) const {
ScriptWrappable::Trace(visitor);
visitor->Trace(ec_);
visitor->Trace(remote_);
}

void PendingBeacon::setData(
const V8UnionReadableStreamOrXMLHttpRequestBodyInit* data) {
if (method_ == http_names::kGET) {
// TODO(crbug.com/1293679): Throw errors.
return;
}
using ContentType =
V8UnionReadableStreamOrXMLHttpRequestBodyInit::ContentType;
switch (data->GetContentType()) {
case ContentType::kUSVString: {
SetDataInternal(BeaconString(data->GetAsUSVString()));
return;
}
case ContentType::kArrayBuffer: {
SetDataInternal(BeaconDOMArrayBuffer(data->GetAsArrayBuffer()));
return;
}
case ContentType::kArrayBufferView: {
SetDataInternal(
BeaconDOMArrayBufferView(data->GetAsArrayBufferView().Get()));
return;
}
case ContentType::kFormData: {
SetDataInternal(BeaconFormData(data->GetAsFormData()));
return;
}
case ContentType::kURLSearchParams: {
SetDataInternal(BeaconURLSearchParams(data->GetAsURLSearchParams()));
return;
}
case ContentType::kBlob:
// TODO(crbug.com/1293679): Decide whether to support blob/file.
case ContentType::kReadableStream: {
// TODO(crbug.com/1293679): Throw errors.
break;
}
}
NOTIMPLEMENTED();
}

void PendingBeacon::deactivate() {
remote_->Deactivate();
if (pending_) {
remote_->Deactivate();
pending_ = false;
}
}

void PendingBeacon::sendNow() {
if (is_pending_) {
if (pending_) {
remote_->SendNow();
is_pending_ = false;
pending_ = false;
}
}

void PendingBeacon::setBackgroundTimeout(int32_t background_timeout) {
background_timeout_ = base::Milliseconds(background_timeout);
}

void PendingBeacon::setTimeout(int32_t timeout) {
timeout_ = base::Milliseconds(timeout);
}

void PendingBeacon::SetURLInternal(const String& url) {
url_ = url;
KURL host_url = ec_->CompleteURL(url);
remote_->SetRequestURL(host_url);
}

void PendingBeacon::SetDataInternal(const BeaconData& data) {
ResourceRequest request;

Expand Down

0 comments on commit d65b747

Please sign in to comment.