New issue
Advanced search Search tips

Issue 922195 link

Starred by 3 users
Status: Assigned
Owner:
npm@chromium.org
Cc:
yoavweiss@chromium.org
lfg@chromium.org
panicker@chromium.org
npm@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

PerformanceObserver.supportedEntryTypes doesn't understand origin trials

Project Member Reported by skobes@chromium.org, Jan 15 
Some entry types are supported when an origin trial is enabled.  These aren't correctly reflected by the supportedEntryTypes attribute.

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/timing/performance_observer.cc?q=PerformanceObserver::supportedEntryTypes

In principle we should be able to query the origin trial's status on the execution context of the script that is reading the attribute.  But it might require some change to the way the bindings code handles FrozenArray.

Comment 1 by npm@chromium.org, Jan 15

Cc: yoavweiss@chromium.org
I think the problem is more that the method is static (I think FrozenArray does not imply static). Yoav, should we change the type of supportedEntryTypes to not be static so that we are allowed to return different arrays in different windows?

Comment 2 by skobes@chromium.org, Jan 15 

The call site is in generated code, so something is making it static.

Comment 4 by jbroman@google.com, Jan 15 

In Blink you should be able to use CallWith=ScriptState to get access to the ScriptState which should be enough to determine what execution context you're in (and thus what should be exposed).

Comment 5 by skobes@chromium.org, Jan 15 

To be clear - I think supportedEntryTypes should remain static in Javascript.  That's the right thing for ergonomics.

The question is how to implement it on the C++ side in a way that is specific to the window.  Is it not possible to know which window's PerformanceObserver class the attribute is being read from?

Comment 6 by jbroman@chromium.org, Jan 15 

#5: See #4. :)

Comment 7 by skobes@chromium.org, Jan 15 

Thanks, comment #4 addresses #5

Comment 8 by npm@chromium.org, Jan 15 

Ah cool, thanks Jeremy! I'll give it a shot soonish

Comment 9 by npm@chromium.org, Jan 15

Cc: -npm@chromium.org
Owner: npm@chromium.org
Status: Assigned (was: Unconfirmed)
Project Member

Comment 10 by bugdroid1@chromium.org, Yesterday (36 hours ago) 

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

commit f61f302c753a1e4bc2abcb206a984e1b51c8d4b4
Author: Nicolas Pena <npm@chromium.org>
Date: Mon Jan 21 18:12:13 2019

Allow supportedEntryTypes to know its ExecutionContext

This CL adds the ScriptState to PerformanceObserver.upportedEntryTypes.
This allows knowing when an origin trial is on and it also permits
excluding entryTypes such as 'paint' in Worker contexts.

Bug: 922195
Change-Id: I391f62fdd4c39e571010d116f3dfef309450cca0
Reviewed-on: https://chromium-review.googlesource.com/c/1416832
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624627}
[modify] https://crrev.com/f61f302c753a1e4bc2abcb206a984e1b51c8d4b4/third_party/blink/renderer/core/timing/performance_observer.cc
[modify] https://crrev.com/f61f302c753a1e4bc2abcb206a984e1b51c8d4b4/third_party/blink/renderer/core/timing/performance_observer.h
[modify] https://crrev.com/f61f302c753a1e4bc2abcb206a984e1b51c8d4b4/third_party/blink/renderer/core/timing/performance_observer.idl
[modify] https://crrev.com/f61f302c753a1e4bc2abcb206a984e1b51c8d4b4/third_party/blink/web_tests/TestExpectations
►

Sign in to add a comment