New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 742530 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Performance Observer lifetime is confusing

Project Member Reported by panicker@chromium.org, Jul 13 2017

Issue description

Currently if a Performance observer instance is not explicitly kept alive in JS (eg. by attaching to window or long lived object), then it can be GC'd and will stop firing the callback at an arbitrary time.
This is confusing semantics for developers.
We should keep the observer alive until "disconnect()" is called.

 
Labels: Hotlist-PerformanceAPIs
Cc: panicker@chromium.org
Owner: ----
Status: Available (was: Assigned)
Owner: npm@chromium.org
Status: Assigned (was: Available)

Comment 4 by npm@chromium.org, Jul 26 2017

To clarify, do we need to keep it alive even if observe() has not been called yet, or only after that has occurred? Also, note that custom bindings for PerformanceObserver have recently been removed.

Comment 5 by panicker@google.com, Jul 26 2017

It should be kept alive from creation to "disconnect"
Cc: brajkumar@chromium.org
 Issue 734358  has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 3 2017

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

commit 4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d
Author: Nicolas Pena <npm@chromium.org>
Date: Thu Aug 03 17:22:12 2017

Keep PerformanceObserver alive until disconnect is called

This CL makes PerformanceObserver an ActiveScriptWrappable so it is not
destroyed by the garbage collector when it goes out of scope but it's
still observing (i.e. disconnect has not been called). Currently, GC
can destroy it and it will stop firing the callback at an arbitrary
time.

Bug:  chromium:742530 
Change-Id: Iec216f7c870d887f2b258536edd9164e1806c259
Reviewed-on: https://chromium-review.googlesource.com/587596
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491776}
[add] https://crrev.com/4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d/third_party/WebKit/LayoutTests/fast/performance/performance-observer-after-gc.html
[modify] https://crrev.com/4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d/third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp
[modify] https://crrev.com/4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d/third_party/WebKit/Source/core/timing/PerformanceObserver.cpp
[modify] https://crrev.com/4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d/third_party/WebKit/Source/core/timing/PerformanceObserver.h
[modify] https://crrev.com/4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d/third_party/WebKit/Source/core/timing/PerformanceObserver.idl
[modify] https://crrev.com/4735d05b7b1c2b3fff3b4e54ec7ed689ffaae25d/third_party/WebKit/Source/core/timing/PerformanceObserverTest.cpp

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

Status: Fixed (was: Assigned)
Hi,
I tried to find out when this commit will be part of Chromium (and more importantly of Chrome itself) but without any luck (I tried following these steps: https://www.chromium.org/blink/when-will-a-fix-ship-in-chrome-stable-or-canary).
Could you guys give me some directions?

Thx.
This change is in M62, which is already stable.

Sign in to add a comment