Revert "[EventTiming] Ship First Input Timing on main frames"
This reverts commit 1ff422d8a30a34713c652508fc170f75120460a1.
Reason for revert: Will reland shipping First Input Timing once we are sure it's going to ship. For now, let's unship.
Original change's description:
> [EventTiming] Ship First Input Timing on main frames
>
> Intent to ship:
> https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/blink-dev/LN92hkaKCzw/Y-ryVxu5AgAJ
>
> Bug: 823744, 841224
> Change-Id: Ic4d3d67facd0d717b929b9798650eb502057fea6
> Reviewed-on: https://chromium-review.googlesource.com/c/1368826
> Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
> Reviewed-by: Yoav Weiss <yoav@yoav.ws>
> Cr-Commit-Position: refs/heads/master@{#625684}
TBR=yoav@yoav.ws,tdresser@chromium.org,npm@chromium.org,yoavweiss@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 823744, 841224
Change-Id: I4c95a1fd789793c240a35580f612627a0bc7af13
Reviewed-on: https://chromium-review.googlesource.com/c/1460428
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630528}diff --git a/third_party/blink/renderer/core/dom/events/event_dispatcher.cc b/third_party/blink/renderer/core/dom/events/event_dispatcher.cc
index 335c457..e217aa9 100644
--- a/third_party/blink/renderer/core/dom/events/event_dispatcher.cc
+++ b/third_party/blink/renderer/core/dom/events/event_dispatcher.cc
@@ -140,15 +140,14 @@
return DispatchEventResult::kNotCanceled;
}
std::unique_ptr<EventTiming> eventTiming;
- const Document& document = node_->GetDocument();
- LocalFrame* frame = document.GetFrame();
- if (frame && frame->DomWindow()) {
- if (origin_trials::EventTimingEnabled(&document)) {
- UseCounter::Count(document,
+ if (origin_trials::EventTimingEnabled(&node_->GetDocument())) {
+ LocalFrame* frame = node_->GetDocument().GetFrame();
+ if (frame && frame->DomWindow()) {
+ UseCounter::Count(node_->GetDocument(),
WebFeature::kPerformanceEventTimingConstructor);
+ eventTiming = std::make_unique<EventTiming>(frame->DomWindow());
+ eventTiming->WillDispatchEvent(*event_);
}
- eventTiming = std::make_unique<EventTiming>(frame->DomWindow());
- eventTiming->WillDispatchEvent(*event_);
}
event_->GetEventPath().EnsureWindowEventContext();
@@ -156,6 +155,8 @@
event_->IsMouseEvent() && event_->type() == event_type_names::kClick;
if (is_click && event_->isTrusted()) {
+ Document& document = node_->GetDocument();
+ LocalFrame* frame = document.GetFrame();
if (frame) {
// A genuine mouse click cannot be triggered by script so we don't expect
// there are any script in the stack.
diff --git a/third_party/blink/renderer/core/timing/event_timing.cc b/third_party/blink/renderer/core/timing/event_timing.cc
index 3315755..cca33b2 100644
--- a/third_party/blink/renderer/core/timing/event_timing.cc
+++ b/third_party/blink/renderer/core/timing/event_timing.cc
@@ -7,7 +7,6 @@
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/events/pointer_event.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
-#include "third_party/blink/renderer/core/origin_trials/origin_trials.h"
#include "third_party/blink/renderer/core/timing/dom_window_performance.h"
#include "third_party/blink/renderer/core/timing/performance_event_timing.h"
#include "third_party/blink/renderer/platform/wtf/time.h"
@@ -26,29 +25,22 @@
event.isTrusted();
}
-void EventTiming::FinishWillDispatch() {
- processing_start_ = CurrentTimeTicks();
- finished_will_dispatch_event_ = true;
-}
-
void EventTiming::WillDispatchEvent(const Event& event) {
// Assume each event can be dispatched only once.
DCHECK(!finished_will_dispatch_event_);
if (!performance_ || !ShouldReportForEventTiming(event))
return;
- // We care about events when the first input is desired or
- // when the EventTiming origin trial is enabled.
- if (performance_->FirstInputDesired()) {
- FinishWillDispatch();
- return;
- }
- if (!origin_trials::EventTimingEnabled(performance_->GetExecutionContext()))
- return;
+ // Although we screen the events for timing by setting these conditions here,
+ // we cannot assume that the conditions should still hold true in
+ // DidDispatchEvent. These conditions have to be re-tested before an entry is
+ // dispatched.
if ((performance_->ShouldBufferEntries() &&
!performance_->IsEventTimingBufferFull()) ||
- performance_->HasObserverFor(PerformanceEntry::kEvent)) {
- FinishWillDispatch();
+ performance_->HasObserverFor(PerformanceEntry::kEvent) ||
+ !performance_->FirstInputDetected()) {
+ processing_start_ = CurrentTimeTicks();
+ finished_will_dispatch_event_ = true;
}
}
diff --git a/third_party/blink/renderer/core/timing/event_timing.h b/third_party/blink/renderer/core/timing/event_timing.h
index 96648d9..59bacd8 100644
--- a/third_party/blink/renderer/core/timing/event_timing.h
+++ b/third_party/blink/renderer/core/timing/event_timing.h
@@ -26,7 +26,6 @@
void DidDispatchEvent(const Event&);
private:
- void FinishWillDispatch();
bool ShouldReportForEventTiming(const Event& event) const;
// The time the first event handler or default action started to execute.
TimeTicks processing_start_;
diff --git a/third_party/blink/renderer/core/timing/performance_event_timing.idl b/third_party/blink/renderer/core/timing/performance_event_timing.idl
index 951011e3..ef577a5 100644
--- a/third_party/blink/renderer/core/timing/performance_event_timing.idl
+++ b/third_party/blink/renderer/core/timing/performance_event_timing.idl
@@ -3,8 +3,10 @@
// found in the LICENSE file.
// https://github.com/wicg/event-timing
-[Exposed=Window]
-interface PerformanceEventTiming : PerformanceEntry {
+[
+ Exposed=Window,
+ OriginTrialEnabled=EventTiming
+] interface PerformanceEventTiming : PerformanceEntry {
readonly attribute DOMHighResTimeStamp processingStart;
readonly attribute DOMHighResTimeStamp processingEnd;
readonly attribute boolean cancelable;
diff --git a/third_party/blink/renderer/core/timing/performance_observer.cc b/third_party/blink/renderer/core/timing/performance_observer.cc
index 5850cc8..1cb5429 100644
--- a/third_party/blink/renderer/core/timing/performance_observer.cc
+++ b/third_party/blink/renderer/core/timing/performance_observer.cc
@@ -59,9 +59,10 @@
if (execution_context->IsDocument()) {
if (origin_trials::ElementTimingEnabled(execution_context))
supportedEntryTypes.push_back(performance_entry_names::kElement);
- if (origin_trials::EventTimingEnabled(execution_context))
+ if (origin_trials::EventTimingEnabled(execution_context)) {
supportedEntryTypes.push_back(performance_entry_names::kEvent);
- supportedEntryTypes.push_back(performance_entry_names::kFirstInput);
+ supportedEntryTypes.push_back(performance_entry_names::kFirstInput);
+ }
if (origin_trials::LayoutJankAPIEnabled(execution_context))
supportedEntryTypes.push_back(performance_entry_names::kLayoutJank);
supportedEntryTypes.push_back(performance_entry_names::kLongtask);
diff --git a/third_party/blink/renderer/core/timing/window_performance.cc b/third_party/blink/renderer/core/timing/window_performance.cc
index 89924e9..3f264bb5 100644
--- a/third_party/blink/renderer/core/timing/window_performance.cc
+++ b/third_party/blink/renderer/core/timing/window_performance.cc
@@ -329,23 +329,12 @@
return !timing() || !timing()->loadEventStart();
}
-bool WindowPerformance::FirstInputDesired() const {
- if (!GetFrame() || !GetFrame()->IsMainFrame())
- return false;
-
- return !first_input_timing_;
-}
-
void WindowPerformance::RegisterEventTiming(const AtomicString& event_type,
TimeTicks start_time,
TimeTicks processing_start,
TimeTicks processing_end,
bool cancelable) {
- if (!GetFrame())
- return;
-
- DCHECK(FirstInputDesired() ||
- origin_trials::EventTimingEnabled(GetExecutionContext()));
+ DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
// |start_time| could be null in some tests that inject input.
DCHECK(!processing_start.is_null());
@@ -375,11 +364,7 @@
void WindowPerformance::ReportEventTimings(WebLayerTreeView::SwapResult result,
TimeTicks timestamp) {
- if (!GetFrame())
- return;
-
- DCHECK(FirstInputDesired() ||
- origin_trials::EventTimingEnabled(GetExecutionContext()));
+ DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
DOMHighResTimeStamp end_time = MonotonicTimeToDOMHighResTimeStamp(timestamp);
for (const auto& entry : event_timings_) {
@@ -397,11 +382,8 @@
PerformanceEventTiming::CreateFirstInputTiming(entry));
}
}
- // Do not report EventTiming unless origin trial is enabled!
- if (!origin_trials::EventTimingEnabled(GetExecutionContext()) ||
- duration_in_ms <= kEventTimingDurationThresholdInMs) {
+ if (duration_in_ms <= kEventTimingDurationThresholdInMs)
continue;
- }
if (HasObserverFor(PerformanceEntry::kEvent)) {
UseCounter::Count(GetFrame(),
@@ -432,6 +414,8 @@
void WindowPerformance::DispatchFirstInputTiming(
PerformanceEventTiming* entry) {
+ DCHECK(origin_trials::EventTimingEnabled(GetExecutionContext()));
+
if (!entry)
return;
DCHECK_EQ("firstInput", entry->entryType());
diff --git a/third_party/blink/renderer/core/timing/window_performance.h b/third_party/blink/renderer/core/timing/window_performance.h
index 2bbc08c..d4182035 100644
--- a/third_party/blink/renderer/core/timing/window_performance.h
+++ b/third_party/blink/renderer/core/timing/window_performance.h
@@ -69,10 +69,7 @@
bool ShouldBufferEntries();
- // Currently, returns false always for frames that are not main frames because
- // we're only shipping FirstInputTiming on main frames. For main frames,
- // returns true if the first input has not yet been calculated.
- bool FirstInputDesired() const;
+ bool FirstInputDetected() const { return !!first_input_timing_; }
// This method creates a PerformanceEventTiming and if needed creates a swap
// promise to calculate the |duration| attribute when such promise is
diff --git a/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt b/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt
index 827521e..44b5356 100644
--- a/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt
+++ b/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt
@@ -4536,13 +4536,6 @@
getter startTime
method constructor
method toJSON
-interface PerformanceEventTiming : PerformanceEntry
- attribute @@toStringTag
- getter cancelable
- getter processingEnd
- getter processingStart
- method constructor
- method toJSON
interface PerformanceLongTaskTiming : PerformanceEntry
attribute @@toStringTag
getter attribution
