New issue
Advanced search Search tips

Issue 735543 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Task



Sign in to add a comment

Performance.idl Contains Interfaces that are Out of Spec and Not Implemented

Project Member Reported by jmedley@google.com, Jun 21 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce the problem:
Compare the Performance.idl to the spec.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/Performance.idl?l=67

https://wicg.github.io/frame-timing/#performanceframetiming-interface

What is the expected behavior?

What went wrong?
It appears that the idl members were never removed when the spec was updated.

Did this work before? N/A 

Chrome version:   Channel: n/a
OS Version: OS X 10.12.5
Flash Version: 

Email discussions with Ilya Grigorik and Shubhie Panicker suggested that these idl members, and any related code should be removed. Shubhie asked that I file this ticket.
 
Labels: -Type-Bug -OS-Mac OS-All Type-Task
Cc: npm@chromium.org tdres...@chromium.org
Components: -Blink Blink>PerformanceAPIs
Status: Available (was: Unconfirmed)
In particular, we should remove 
clearFrameTimings
setFrameTimingBufferSize
onframetimingbufferfull

Comment 3 by jmedley@google.com, Jun 21 2017

Meant to list those and got distracted. Sorry.

Comment 4 by npm@chromium.org, Jun 23 2017

Cc: -npm@chromium.org
Owner: npm@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 7 2017

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

commit 908c23fcafc7ad610e0633ee2997deca7c462736
Author: npm <npm@chromium.org>
Date: Fri Jul 07 04:01:45 2017

Remove out of spec FrameTiming members from Performance.idl

BUG= 735543 

Review-Url: https://codereview.chromium.org/2952413002
Cr-Commit-Position: refs/heads/master@{#484827}

[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-properties-after-frame-navigated-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-properties-after-frame-removed-and-gced-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-properties-after-frame-removed-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-navigated-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/fast/dom/Window/window-properties-performance-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/Source/core/events/EventTypeNames.json5
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/Source/core/timing/Performance.idl
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
[modify] https://crrev.com/908c23fcafc7ad610e0633ee2997deca7c462736/third_party/WebKit/Source/core/timing/PerformanceBase.h

Comment 6 by npm@chromium.org, Jul 7 2017

Status: Fixed (was: Started)

Comment 7 by npm@chromium.org, Jul 18 2017

Cc: jmedley@google.com
Status: Assigned (was: Fixed)
Actually I was not aware of the 'Intent to Remove' needed for these kinds of things. Is there currently any such intent? Otherwise I'll revert this and file the intent, and afterwards reland...

Comment 8 by npm@chromium.org, Jul 18 2017

Status: Fixed (was: Assigned)
Never mind, spoke with rbyers@ and since they were under RuntimeEnabled=FrameTimingSupport which has experimental status [1] then no need for that in this case.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5?type=cs&q=FrameTimingSupport&sq=package:chromium&l=480
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 27 2017

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

commit e5c1255633df723a92f181c5d5c3162131db8449
Author: Nicolas Pena <npm@chromium.org>
Date: Thu Jul 27 15:27:22 2017

Remove unused FrameTimingSupport feature

Bug:  chromium:735543 
Change-Id: I63a84209e3c5ae518e01a6eff1e6302a5fb43575
Reviewed-on: https://chromium-review.googlesource.com/587628
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Nicolás Peña <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489932}
[modify] https://crrev.com/e5c1255633df723a92f181c5d5c3162131db8449/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

Sign in to add a comment