Commit 88727c8e authored by Yuta Kasai's avatar Yuta Kasai Committed by Commit Bot

ResourceTiming: Enable FetchEventWorkerTiming for subresources

This patch is a part of FetchEvent WorkerTiming patches. This feature enables a
service worker to attach PerformanceMark/PerformanceMeasure timings to a
request during the fetch event handler. The timings will then be exposed to the
page using blink::PerformanceResourceTiming.

This patch passes a Mojo pending receiver for WorkerTimingContainer to
blink::PerformanceResourceTiming by TakePendingWorkerTimingReceiver() when the
request is completed. In order to achieve it, this patch adds receiver to
blink::ResourceTimingInfo as a mutable member variable and extends all
AddResourceTiming() to pass receivers.

The reason of the mutable variable is that it must be passed to
blink::PerformanceResourceTiming by move semantics but blink::ResourceTimingInfo
is passed only by const reference. blink::ResourceTimingInfo can't be changed to
pass by value because blink::ResourceTimingInfo is created by scoped_refptr and
it passes its pointer to PerformanceNavigationTiming now.

Explainer : https://github.com/wanderview/fetchevent-worker-timing/blob/master/explainer.md
Design doc: https://docs.google.com/document/d/1-ebnv7OFiVd3k2-jbtQGO5s3BKHIp7lRx3ujhgNKvB0

Bug: 900700
Change-Id: I04bee9dc1990564db3c102b5f2ce64266cb76a2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1923869
Commit-Queue: Yuta Kasai <yutakasai@google.com>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Reviewed-by: default avatarNicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719091}
parent 4d3c5a51
...@@ -313,8 +313,11 @@ void WebRemoteFrameImpl::ForwardResourceTimingToParent( ...@@ -313,8 +313,11 @@ void WebRemoteFrameImpl::ForwardResourceTimingToParent(
HTMLFrameOwnerElement* owner_element = HTMLFrameOwnerElement* owner_element =
To<HTMLFrameOwnerElement>(frame_->Owner()); To<HTMLFrameOwnerElement>(frame_->Owner());
DCHECK(owner_element); DCHECK(owner_element);
// TODO(https://crbug.com/900700): Take a Mojo pending receiver for
// WorkerTimingContainer for navigation from the calling function.
DOMWindowPerformance::performance(*parent_frame->GetFrame()->DomWindow()) DOMWindowPerformance::performance(*parent_frame->GetFrame()->DomWindow())
->AddResourceTiming(info, owner_element->localName()); ->AddResourceTiming(info, owner_element->localName(),
mojo::NullReceiver() /* worker_timing_receiver */);
} }
void WebRemoteFrameImpl::SetNeedsOcclusionTracking(bool needs_tracking) { void WebRemoteFrameImpl::SetNeedsOcclusionTracking(bool needs_tracking) {
......
...@@ -642,6 +642,9 @@ void DocumentLoader::BodyLoadingFinished( ...@@ -642,6 +642,9 @@ void DocumentLoader::BodyLoadingFinished(
// which will be fixed with synchronous commit. // which will be fixed with synchronous commit.
// Main resource timing information is reported through the owner // Main resource timing information is reported through the owner
// to be passed to the parent frame, if appropriate. // to be passed to the parent frame, if appropriate.
// TODO(https://crbug.com/900700): Set a Mojo pending receiver for
// WorkerTimingContainer in |navigation_timing_info|.
frame_->Owner()->AddResourceTiming(*navigation_timing_info_); frame_->Owner()->AddResourceTiming(*navigation_timing_info_);
} }
frame_->SetShouldSendResourceTimingInfoToParent(false); frame_->SetShouldSendResourceTimingInfoToParent(false);
......
...@@ -228,7 +228,11 @@ void WorkerFetchContext::AddResourceTiming(const ResourceTimingInfo& info) { ...@@ -228,7 +228,11 @@ void WorkerFetchContext::AddResourceTiming(const ResourceTimingInfo& info) {
.GetSecurityOrigin(); .GetSecurityOrigin();
WebResourceTimingInfo web_info = Performance::GenerateResourceTiming( WebResourceTimingInfo web_info = Performance::GenerateResourceTiming(
*security_origin, info, *global_scope_); *security_origin, info, *global_scope_);
resource_timing_notifier_->AddResourceTiming(web_info, info.InitiatorType()); // |info| is taken const-ref but this can make destructive changes to
// WorkerTimingContainer on |info| when a page is controlled by a service
// worker.
resource_timing_notifier_->AddResourceTiming(web_info, info.InitiatorType(),
info.TakeWorkerTimingReceiver());
} }
void WorkerFetchContext::PopulateResourceRequest( void WorkerFetchContext::PopulateResourceRequest(
......
...@@ -64,34 +64,40 @@ WorkerResourceTimingNotifierImpl::WorkerResourceTimingNotifierImpl( ...@@ -64,34 +64,40 @@ WorkerResourceTimingNotifierImpl::WorkerResourceTimingNotifierImpl(
void WorkerResourceTimingNotifierImpl::AddResourceTiming( void WorkerResourceTimingNotifierImpl::AddResourceTiming(
const WebResourceTimingInfo& info, const WebResourceTimingInfo& info,
const AtomicString& initiator_type) { const AtomicString& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver) {
if (task_runner_->RunsTasksInCurrentSequence()) { if (task_runner_->RunsTasksInCurrentSequence()) {
DCHECK(inside_execution_context_); DCHECK(inside_execution_context_);
if (inside_execution_context_->IsContextDestroyed()) if (inside_execution_context_->IsContextDestroyed())
return; return;
DCHECK(inside_execution_context_->IsContextThread()); DCHECK(inside_execution_context_->IsContextThread());
GetPerformance(*inside_execution_context_) GetPerformance(*inside_execution_context_)
->AddResourceTiming(info, initiator_type); ->AddResourceTiming(info, initiator_type,
std::move(worker_timing_receiver));
} else { } else {
PostCrossThreadTask( PostCrossThreadTask(
*task_runner_, FROM_HERE, *task_runner_, FROM_HERE,
CrossThreadBindOnce( CrossThreadBindOnce(
&WorkerResourceTimingNotifierImpl::AddCrossThreadResourceTiming, &WorkerResourceTimingNotifierImpl::AddCrossThreadResourceTiming,
WrapCrossThreadWeakPersistent(this), info, WrapCrossThreadWeakPersistent(this), info,
initiator_type.GetString())); initiator_type.GetString(), std::move(worker_timing_receiver)));
} }
} }
void WorkerResourceTimingNotifierImpl::AddCrossThreadResourceTiming( void WorkerResourceTimingNotifierImpl::AddCrossThreadResourceTiming(
const WebResourceTimingInfo& info, const WebResourceTimingInfo& info,
const String& initiator_type) { const String& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver) {
DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (!outside_execution_context_ || if (!outside_execution_context_ ||
outside_execution_context_->IsContextDestroyed()) outside_execution_context_->IsContextDestroyed())
return; return;
DCHECK(outside_execution_context_->IsContextThread()); DCHECK(outside_execution_context_->IsContextThread());
GetPerformance(*outside_execution_context_) GetPerformance(*outside_execution_context_)
->AddResourceTiming(info, AtomicString(initiator_type)); ->AddResourceTiming(info, AtomicString(initiator_type),
std::move(worker_timing_receiver));
} }
void WorkerResourceTimingNotifierImpl::Trace(Visitor* visitor) { void WorkerResourceTimingNotifierImpl::Trace(Visitor* visitor) {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "third_party/blink/public/mojom/timing/worker_timing_container.mojom-blink.h"
#include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/heap/persistent.h" #include "third_party/blink/renderer/platform/heap/persistent.h"
...@@ -37,14 +38,20 @@ class CORE_EXPORT WorkerResourceTimingNotifierImpl final ...@@ -37,14 +38,20 @@ class CORE_EXPORT WorkerResourceTimingNotifierImpl final
scoped_refptr<base::SingleThreadTaskRunner> task_runner); scoped_refptr<base::SingleThreadTaskRunner> task_runner);
~WorkerResourceTimingNotifierImpl() override = default; ~WorkerResourceTimingNotifierImpl() override = default;
void AddResourceTiming(const WebResourceTimingInfo&, void AddResourceTiming(
const AtomicString& initiator_type) override; const WebResourceTimingInfo&,
const AtomicString& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver) override;
void Trace(Visitor*) override; void Trace(Visitor*) override;
private: private:
void AddCrossThreadResourceTiming(const WebResourceTimingInfo&, void AddCrossThreadResourceTiming(
const String& initiator_type); const WebResourceTimingInfo&,
const String& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver);
scoped_refptr<base::SingleThreadTaskRunner> task_runner_; scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
...@@ -72,8 +79,11 @@ class CORE_EXPORT NullWorkerResourceTimingNotifier final ...@@ -72,8 +79,11 @@ class CORE_EXPORT NullWorkerResourceTimingNotifier final
NullWorkerResourceTimingNotifier() = default; NullWorkerResourceTimingNotifier() = default;
~NullWorkerResourceTimingNotifier() override = default; ~NullWorkerResourceTimingNotifier() override = default;
void AddResourceTiming(const WebResourceTimingInfo&, void AddResourceTiming(
const AtomicString& initiator_type) override {} const WebResourceTimingInfo&,
const AtomicString& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver) override {}
private: private:
DISALLOW_COPY_AND_ASSIGN(NullWorkerResourceTimingNotifier); DISALLOW_COPY_AND_ASSIGN(NullWorkerResourceTimingNotifier);
......
...@@ -35,6 +35,7 @@ ...@@ -35,6 +35,7 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/time/default_clock.h" #include "base/time/default_clock.h"
#include "base/time/default_tick_clock.h" #include "base/time/default_tick_clock.h"
#include "third_party/blink/public/mojom/timing/worker_timing_container.mojom-blink-forward.h"
#include "third_party/blink/public/platform/platform.h" #include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h" #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h" #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
...@@ -450,9 +451,13 @@ void Performance::GenerateAndAddResourceTiming( ...@@ -450,9 +451,13 @@ void Performance::GenerateAndAddResourceTiming(
const SecurityOrigin* security_origin = GetSecurityOrigin(context); const SecurityOrigin* security_origin = GetSecurityOrigin(context);
if (!security_origin) if (!security_origin)
return; return;
// |info| is taken const-ref but this can make destructive changes to
// WorkerTimingContainer on |info| when a page is controlled by a service
// worker.
AddResourceTiming( AddResourceTiming(
GenerateResourceTiming(*security_origin, info, *context), GenerateResourceTiming(*security_origin, info, *context),
!initiator_type.IsNull() ? initiator_type : info.InitiatorType()); !initiator_type.IsNull() ? initiator_type : info.InitiatorType(),
info.TakeWorkerTimingReceiver());
} }
WebResourceTimingInfo Performance::GenerateResourceTiming( WebResourceTimingInfo Performance::GenerateResourceTiming(
...@@ -526,13 +531,13 @@ WebResourceTimingInfo Performance::GenerateResourceTiming( ...@@ -526,13 +531,13 @@ WebResourceTimingInfo Performance::GenerateResourceTiming(
return result; return result;
} }
void Performance::AddResourceTiming(const WebResourceTimingInfo& info, void Performance::AddResourceTiming(
const AtomicString& initiator_type) { const WebResourceTimingInfo& info,
// TODO(https://crbug.com/900700): Implement this, making the receiver const AtomicString& initiator_type,
// connected to a remote which is passed to blink::FetchEvent for subresouces mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
// and finding the path for navigation. worker_timing_receiver) {
auto* entry = MakeGarbageCollected<PerformanceResourceTiming>( auto* entry = MakeGarbageCollected<PerformanceResourceTiming>(
info, time_origin_, initiator_type, mojo::NullReceiver()); info, time_origin_, initiator_type, std::move(worker_timing_receiver));
NotifyObserversOfEntry(*entry); NotifyObserversOfEntry(*entry);
// https://w3c.github.io/resource-timing/#dfn-add-a-performanceresourcetiming-entry // https://w3c.github.io/resource-timing/#dfn-add-a-performanceresourcetiming-entry
if (CanAddResourceTimingEntry() && if (CanAddResourceTimingEntry() &&
......
...@@ -164,8 +164,11 @@ class CORE_EXPORT Performance : public EventTargetWithInlineData { ...@@ -164,8 +164,11 @@ class CORE_EXPORT Performance : public EventTargetWithInlineData {
const SecurityOrigin& destination_origin, const SecurityOrigin& destination_origin,
const ResourceTimingInfo&, const ResourceTimingInfo&,
ExecutionContext& context_for_use_counter); ExecutionContext& context_for_use_counter);
void AddResourceTiming(const WebResourceTimingInfo&, void AddResourceTiming(
const AtomicString& initiator_type); const WebResourceTimingInfo&,
const AtomicString& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver);
void NotifyNavigationTimingToObservers(); void NotifyNavigationTimingToObservers();
......
...@@ -81,10 +81,13 @@ PerformanceResourceTiming::PerformanceResourceTiming( ...@@ -81,10 +81,13 @@ PerformanceResourceTiming::PerformanceResourceTiming(
allow_negative_value_(info.allow_negative_values), allow_negative_value_(info.allow_negative_values),
is_secure_context_(info.is_secure_context), is_secure_context_(info.is_secure_context),
server_timing_( server_timing_(
PerformanceServerTiming::FromParsedServerTiming(info.server_timing)) { PerformanceServerTiming::FromParsedServerTiming(info.server_timing)),
} worker_timing_receiver_(this, std::move(worker_timing_receiver)) {}
// This constructor is for PerformanceNavigationTiming. // This constructor is for PerformanceNavigationTiming.
// TODO(https://crbug.com/900700): Set a Mojo pending receiver for
// WorkerTimingContainer in |worker_timing_receiver_| when a service worker
// controls a page.
PerformanceResourceTiming::PerformanceResourceTiming( PerformanceResourceTiming::PerformanceResourceTiming(
const AtomicString& name, const AtomicString& name,
base::TimeTicks time_origin, base::TimeTicks time_origin,
...@@ -94,7 +97,8 @@ PerformanceResourceTiming::PerformanceResourceTiming( ...@@ -94,7 +97,8 @@ PerformanceResourceTiming::PerformanceResourceTiming(
time_origin_(time_origin), time_origin_(time_origin),
is_secure_context_(is_secure_context), is_secure_context_(is_secure_context),
server_timing_( server_timing_(
PerformanceServerTiming::FromParsedServerTiming(server_timing)) {} PerformanceServerTiming::FromParsedServerTiming(server_timing)),
worker_timing_receiver_(this, mojo::NullReceiver()) {}
PerformanceResourceTiming::~PerformanceResourceTiming() = default; PerformanceResourceTiming::~PerformanceResourceTiming() = default;
......
...@@ -135,6 +135,11 @@ class CORE_EXPORT PerformanceResourceTiming ...@@ -135,6 +135,11 @@ class CORE_EXPORT PerformanceResourceTiming
bool is_secure_context_ = false; bool is_secure_context_ = false;
HeapVector<Member<PerformanceServerTiming>> server_timing_; HeapVector<Member<PerformanceServerTiming>> server_timing_;
HeapVector<Member<PerformanceEntry>> worker_timing_; HeapVector<Member<PerformanceEntry>> worker_timing_;
// Used for getting entries from a service worker to add to
// PerformanceResourceTiming#workerTiming. Null when no service worker handles
// a request for the resource.
mojo::Receiver<mojom::blink::WorkerTimingContainer> worker_timing_receiver_;
}; };
} // namespace blink } // namespace blink
......
...@@ -99,6 +99,12 @@ class PLATFORM_EXPORT FetchContext : public GarbageCollected<FetchContext> { ...@@ -99,6 +99,12 @@ class PLATFORM_EXPORT FetchContext : public GarbageCollected<FetchContext> {
WebScopedVirtualTimePauser& virtual_time_pauser, WebScopedVirtualTimePauser& virtual_time_pauser,
ResourceType); ResourceType);
// WARNING: |info| can be modified by the implementation of this method
// despite the fact that it is given as const-ref. Namely, if
// |worker_timing_receiver_| is set implementations may take (move out) the
// field.
// TODO(shimazu): Fix this. Eventually ResourceTimingInfo should become a mojo
// struct and this should take a moved-value of it.
virtual void AddResourceTiming(const ResourceTimingInfo&); virtual void AddResourceTiming(const ResourceTimingInfo&);
virtual bool AllowImage(bool, const KURL&) const { return false; } virtual bool AllowImage(bool, const KURL&) const { return false; }
virtual base::Optional<ResourceRequestBlockedReason> CanRequest( virtual base::Optional<ResourceRequestBlockedReason> CanRequest(
......
...@@ -1816,9 +1816,9 @@ void ResourceFetcher::HandleLoaderFinish(Resource* resource, ...@@ -1816,9 +1816,9 @@ void ResourceFetcher::HandleLoaderFinish(Resource* resource,
info->AddFinalTransferSize( info->AddFinalTransferSize(
encoded_data_length == -1 ? 0 : encoded_data_length); encoded_data_length == -1 ? 0 : encoded_data_length);
// TODO(https://crbug.com/900700): Get |request_id| from ResourceResponse auto receiver = Context().TakePendingWorkerTimingReceiver(
// through |resource| and take a Mojo pending receiver for resource->GetResponse().RequestId());
// WorkerTimingContainer which is set on |info|. info->SetWorkerTimingReceiver(std::move(receiver));
if (resource->Options().request_initiator_context == kDocumentContext) if (resource->Options().request_initiator_context == kDocumentContext)
Context().AddResourceTiming(*info); Context().AddResourceTiming(*info);
} }
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include <memory> #include <memory>
#include "base/macros.h" #include "base/macros.h"
#include "third_party/blink/public/mojom/timing/worker_timing_container.mojom-blink.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_request.h" #include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_response.h" #include "third_party/blink/renderer/platform/loader/fetch/resource_response.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
...@@ -88,6 +89,17 @@ class PLATFORM_EXPORT ResourceTimingInfo ...@@ -88,6 +89,17 @@ class PLATFORM_EXPORT ResourceTimingInfo
} }
bool NegativeAllowed() const { return negative_allowed_; } bool NegativeAllowed() const { return negative_allowed_; }
void SetWorkerTimingReceiver(
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver) {
worker_timing_receiver_ = std::move(worker_timing_receiver);
}
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
TakeWorkerTimingReceiver() const {
return std::move(worker_timing_receiver_);
}
private: private:
ResourceTimingInfo(const AtomicString& type, const base::TimeTicks time) ResourceTimingInfo(const AtomicString& type, const base::TimeTicks time)
: type_(type), initial_time_(time) {} : type_(type), initial_time_(time) {}
...@@ -102,6 +114,16 @@ class PLATFORM_EXPORT ResourceTimingInfo ...@@ -102,6 +114,16 @@ class PLATFORM_EXPORT ResourceTimingInfo
bool has_cross_origin_redirect_ = false; bool has_cross_origin_redirect_ = false;
bool negative_allowed_ = false; bool negative_allowed_ = false;
// Mutable since it must be passed to blink::PerformanceResourceTiming by move
// semantics but ResourceTimingInfo is passed only by const reference.
// ResourceTimingInfo can't be changed to pass by value because it can
// actually be a large object.
// It can be null when service worker doesn't serve a response for the
// resource. In that case, PerformanceResourceTiming#workerTiming is kept
// empty.
mutable mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver_;
DISALLOW_COPY_AND_ASSIGN(ResourceTimingInfo); DISALLOW_COPY_AND_ASSIGN(ResourceTimingInfo);
}; };
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_WORKER_RESOURCE_TIMING_NOTIFIER_H_ #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_WORKER_RESOURCE_TIMING_NOTIFIER_H_
#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_WORKER_RESOURCE_TIMING_NOTIFIER_H_ #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_WORKER_RESOURCE_TIMING_NOTIFIER_H_
#include "third_party/blink/public/mojom/timing/worker_timing_container.mojom-blink-forward.h"
#include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/heap/persistent.h" #include "third_party/blink/renderer/platform/heap/persistent.h"
...@@ -25,8 +26,11 @@ class WorkerResourceTimingNotifier ...@@ -25,8 +26,11 @@ class WorkerResourceTimingNotifier
// Implementations should route the info to an appropriate Performance // Implementations should route the info to an appropriate Performance
// Timeline which may be associated with a different thread from the current // Timeline which may be associated with a different thread from the current
// running thread. // running thread.
virtual void AddResourceTiming(const WebResourceTimingInfo&, virtual void AddResourceTiming(
const AtomicString& initiator_type) = 0; const WebResourceTimingInfo&,
const AtomicString& initiator_type,
mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
worker_timing_receiver) = 0;
virtual void Trace(Visitor*) {} virtual void Trace(Visitor*) {}
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment