New issue
Advanced search Search tips

Issue 707392 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 674593


Show other hotlists

Hotlists containing this issue:
Non-Standard-IDL


Sign in to add a comment

Merge WorkerPerformance into Performance

Project Member Reported by lunalu@chromium.org, Mar 31 2017

Issue description

As titled.
 
Components: -Blink Blink>PerformanceAPIs
https://w3c.github.io/hr-time/#the-performance-interface is the spec for this.

Comment 2 by panicker@google.com, Apr 13 2017

Status: Available (was: Untriaged)
Owner: npm@chromium.org
Status: Assigned (was: Available)
Nicolas was looking at this a bit, if I remember correctly.

Over to him, but low priority.

Comment 4 by npm@chromium.org, Aug 11 2017

I think we need to wait before we can do this. We currently have Resource + Navigation Timing on Performance but not on WorkerPerformance, and since the plan is to eventually deprecate them we don't want to expose them on Workers.

Comment 5 by npm@chromium.org, Aug 11 2017

Sorry, I meant PerformanceTiming, PerformanceNavigation.
Cc: igrigo...@chromium.org panicker@chromium.org
+panicker, igrigorik to comment on #4/5.

Do we have a clear path to deprecation for these APIs?

From https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md
"Exposed can also be specified with a method, attribute and constant."

I'm not sure what happens if you specify Exposed on the interface and an attribute, but it's possible you could merge the two files, and give PerformanceTiming & PerformanceNavigation overrides to only be exposed on Window.

Comment 7 by foolip@chromium.org, Aug 17 2017

npm@, for a case like that, adding [Exposed=Window] for those attributes would work, if that's the only blocker.

Comment 8 by npm@chromium.org, Aug 24 2017

I tried this again and found 2 problems:
* |readonly attribute MemoryInfo memory| is exposed in Window but under RuntimeEnabled=MemoryInfoInWorkers (experimental) in Worker. Not sure how to say "this is exposed for window, and exposed for worker if MemoryInfoInWorkers is enabled".
* Currently WorkerPerformance is a member of WorkerGlobalScope (not of Worker), so exposing Performance.idl to Worker causes webexposed tests for shared and dedicated workers to fail.
Ping!

Philip, any thoughts on the correct approach, given #8?
#8, what do the new failed webexposed tests look like? If it's just exposing some interfaces, that sounds right.

As for the RuntimeEnabled=MemoryInfoInWorkers problem, yeah, I don't think that's fixable without changing the bindings generator. What's the timeline on launching MemoryInfoInWorkers?
We don't have a concrete timeline.

From my perspective, ideally, we come up with a more standardizable approach, and never ship MemoryInfoInWorkers.

It's unclear if or when this will happen though.

Comment 12 by npm@chromium.org, Feb 1 2018

Yea IIRC the failing webexposed tests were just about exposing interfaces. If we plan to keep MemoryInfo for Performance in Windows but never expose it for Performance in Workers, then this is WontFix in the sense they can't be fully merged.

Comment 13 by npm@chromium.org, Feb 1 2018

Per chat with tdresser, maybe we can remove the option of exposing MemoryInfo in Worker (and remove the MemoryInfoInWorkers flag), and then just have an attribute that is only exposed to Window (I think that should work?).
[Exposed=Window] would work for that, yes.
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ed3245d25d5fa2306abecc5e26d79757525ffd1

commit 0ed3245d25d5fa2306abecc5e26d79757525ffd1
Author: Nicolas Pena <npm@chromium.org>
Date: Fri Feb 16 04:39:26 2018

Rename Performance to WindowPerformance

This CL is the first step in merging the Performance and WorkerPerformance idls.
The following are the full set of steps:
1. Rename Performance.h to WindowPerformance.h.
2. Rename PerformanceBase.h to Performance.h.
3. Merge Performance.idl and WorkerPerformance.idl.

The following is roughly how it would look like after all steps are completed:
https://chromium-review.googlesource.com/c/chromium/src/+/919983

Bug:  707392 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ib99ade431badca0fad9bf61dbd580f3549cbab91
Reviewed-on: https://chromium-review.googlesource.com/922927
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537203}
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/dom/events/Event.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/exported/WebPerformance.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/exported/WebRemoteFrameImpl.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/frame/PerformanceMonitor.h
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/intersection_observer/IntersectionObserver.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/paint/PaintTiming.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/BUILD.gn
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/DOMWindowPerformance.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/DOMWindowPerformance.h
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/Performance.idl
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/PerformanceObserver.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/PerformanceObserverTest.cpp
[rename] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/WindowPerformance.cpp
[rename] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/WindowPerformance.h
[rename] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/core/timing/WindowPerformanceTest.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/modules/mediasource/VideoPlaybackQuality.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/modules/sensor/Sensor.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/Source/modules/webmidi/MIDIOutput.cpp
[modify] https://crrev.com/0ed3245d25d5fa2306abecc5e26d79757525ffd1/third_party/WebKit/public/web/WebPerformance.h

Project Member

Comment 16 by bugdroid1@chromium.org, Feb 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dba58e3ce756f4ccfe79cfe9546521e1f8117536

commit dba58e3ce756f4ccfe79cfe9546521e1f8117536
Author: Nicolas Pena <npm@chromium.org>
Date: Sat Feb 17 05:53:39 2018

Rename PerformanceBase to Performance

This CL is the second step in merging the Performance and WorkerPerformance idls.
The following are the full set of steps:
1. Rename Performance.h to WindowPerformance.h.
2. Rename PerformanceBase.h to Performance.h.
3. Merge Performance.idl and WorkerPerformance.idl.

The following is roughly how it would look like after all steps are completed:
https://chromium-review.googlesource.com/c/chromium/src/+/919983


Bug:  707392 
Change-Id: I1d9d91b29be2910d5a733e685bbccea9180d1d67
Reviewed-on: https://chromium-review.googlesource.com/924276
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537535}
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/dom/IdleDeadline.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/frame/RemoteFrameOwner.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/BUILD.gn
[rename] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/Performance.cpp
[rename] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/Performance.h
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceNavigationTiming.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceObserver.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceObserver.h
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceObserverTest.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceResourceTiming.cpp
[rename] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceTest.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceTiming.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceUserTiming.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/PerformanceUserTiming.h
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/WindowPerformance.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/WindowPerformance.h
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/WindowPerformanceTest.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/WorkerPerformance.cpp
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/core/timing/WorkerPerformance.h
[modify] https://crrev.com/dba58e3ce756f4ccfe79cfe9546521e1f8117536/third_party/WebKit/Source/modules/webmidi/MIDIOutput.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3871f087d726e2f8ea3d95ee254e064913a68440

commit 3871f087d726e2f8ea3d95ee254e064913a68440
Author: Nicolas Pena <npm@chromium.org>
Date: Sat Apr 28 22:10:13 2018

Merge WorkerPerformance.idl into Performance.idl

This CL merges the WorkerPerformance and Performance IDLs. The Exposed=Window
addition to |navigation| and |timing| cause the paths to no longer exist when
the frame is removed (as opposed to exist with trivial 0 values).

Bug:  707392 
Change-Id: I1b8bcfc0731a2075254a727df3f38f5ce3aa5df9
Reviewed-on: https://chromium-review.googlesource.com/1026451
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554656}
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/virtual/stable/http/tests/serviceworker/webexposed/global-interface-listing-service-worker-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-dedicated-worker-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-shared-worker-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-dedicated-worker-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-shared-worker-expected.txt
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/core_idl_files.gni
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/dom_window_performance.cc
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/dom_window_performance.h
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/performance.cc
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/performance.h
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/performance.idl
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/window_performance.cc
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/window_performance.h
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/third_party/blink/renderer/core/timing/worker_performance.h
[delete] https://crrev.com/0ec14c8f9cba13f1bf9fbf8d2ef08bbac6eccc33/third_party/blink/renderer/core/timing/worker_performance.idl
[modify] https://crrev.com/3871f087d726e2f8ea3d95ee254e064913a68440/tools/metrics/histograms/enums.xml

Comment 18 by npm@chromium.org, Apr 30 2018

Status: Fixed (was: Assigned)

Sign in to add a comment