New issue
Advanced search Search tips

Issue 769663 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Add use counters for Streams

Project Member Reported by ricea@chromium.org, Sep 28 2017

Issue description

We need to have some idea of usage.

I think this will require us to export the C++ use counter API on the v8 extras bindings object.

It might be useful to also count the pipeTo and getReader entry points.
 
Which Streams are we talking about here? I could do it but I need a little more context here.

Thanks

Comment 2 by ricea@chromium.org, Feb 15 2018

#1 ReadableStream, WritableStream and TransformStream, defined in https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/streams/

Comment 3 by ricea@chromium.org, Mar 29 2018

Owner: ricea@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 2 2018

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

commit 2f3fc53f816ed090ae5261573e2e65263e21c501
Author: Adam Rice <ricea@chromium.org>
Date: Mon Apr 02 07:25:18 2018

Add use counters for streams

Add use counters to the constructors for ReadableStream, WritableStream
and TransformStream.

A new function, binding.countUse(), is added to the V8 Extras binding
object and called from the constructors.

See the V8 Extras Design Doc for more details on how this works:
https://docs.google.com/document/d/1AT5-T0aHGp7Lt29vPWFr2-qG8r3l9CByyvKwEuA8Ec0/edit#heading=h.334ekprhracb

binding.countUse() is registered by a new C++ function,
AddUseCounterFunctionToV8ExtrasBindings() which is called when setting
up the Javascript contexts for windows and workers.

To help implement tests for the new function, some test functions have
been factored out of core/streams/ReadableStreamOperationsTest.cpp into
V8ExtrasTestUtils.{h,cpp}.

There are also new layout tests to verify that the use counters are
correctly set. Internal creation of a ReadableStream by creating a
Response object does not set the counter.

Currently constructing a TransformStream also set the ReadableStream
constructor use counter. This will be fixed once ReadableStream has been
updated to the latest standard version ( http://crbug.com/710728 ).

Bug:  769663 
Change-Id: I484ef6843cbb800b345d56cf67d09e45b97ab3a3
Reviewed-on: https://chromium-review.googlesource.com/985345
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547420}
[add] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/LayoutTests/http/tests/streams/chromium/use-counters.html
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/bindings.gni
[add] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/InitializeV8ExtrasBinding.cpp
[add] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/InitializeV8ExtrasBinding.h
[add] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/InitializeV8ExtrasBindingTest.cpp
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
[add] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/V8ExtrasTestUtils.cpp
[add] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/V8ExtrasTestUtils.h
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/core/streams/ReadableStream.js
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/core/streams/ReadableStreamOperations.h
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/core/streams/ReadableStreamOperationsTest.cpp
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/core/streams/TransformStream.js
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/Source/core/streams/WritableStream.js
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/2f3fc53f816ed090ae5261573e2e65263e21c501/tools/metrics/histograms/enums.xml

Comment 5 by ricea@chromium.org, Apr 2 2018

Status: Fixed (was: Started)
This is done. I split out issue 828034 to track adding use counters for pipeTo() and getReader().
Great stuff, thank you!!!
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 2 2018

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

commit ec9b78051e623b2e136055025cd1ae3bb56e9724
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Mon Apr 02 19:13:07 2018

[jumbo] rename one of the TryCatchScope's

https://chromium-review.googlesource.com/c/chromium/src/+/985345 added
a second TryCatchScope test helper with different assumptions to the
existing one in bindings/core/v8/ScriptPromiseTest.cpp - which then
broke jumbo builds due to ambiguous namespace lookup results.

Let's rename the TryCatchScope in ScriptPromiseTest.cpp to avoid this
problem.

TBR=yhirano@chromium.org

Bug:  769663 
Change-Id: If9911951669f806a17919c78e1990f3f04209df9
Reviewed-on: https://chromium-review.googlesource.com/989754
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547483}
[modify] https://crrev.com/ec9b78051e623b2e136055025cd1ae3bb56e9724/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseTest.cpp

Sign in to add a comment