Skip to content

GitLab

T
tangled
Commit 2f3fc53f authored by Adam Rice's avatar Adam Rice Committed by Commit Bot
Browse files
Options

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/985345Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547420}
parent e55511b9
Hide whitespace changes
Inline Side-by-side
Showing with 362 additions and 48 deletions
third_party/WebKit/LayoutTests/http/tests/streams/chromium/use-counters.html 0 → 100644
View file @ 2f3fc53f
<!DOCTYPE html>
<meta charset="utf-8">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
'use strict';
// TODO(ricea): Is it possible to just import the generated mojom.js?
const kReadableStreamConstructor = 2399;
const kWritableStreamConstructor = 2400;
const kTransformStreamConstructor = 2401;
// Creating a Response should not trigger the kReadableStreamConstructor
// counter.
test(() => {
new Response('hello');
assert_false(internals.isUseCounted(document, kReadableStreamConstructor),
'use should not have been counted');
}, 'use of Response constructor should not be counted as using the ' +
'ReadableStream constructor');
test(() => {
new ReadableStream();
assert_true(internals.isUseCounted(document, kReadableStreamConstructor),
'use should be counted');
}, 'use of ReadableStream constructor should be counted');
test(() => {
new WritableStream();
assert_true(internals.isUseCounted(document, kWritableStreamConstructor),
'use should be counted');
}, 'use of WritableStream constructor should be counted');
test(() => {
new TransformStream();
assert_true(internals.isUseCounted(document, kTransformStreamConstructor),
'use should be counted');
}, 'use of TransformStream constructor should be counted');
</script>
third_party/WebKit/Source/bindings/bindings.gni
View file @ 2f3fc53f
......@@ -43,6 +43,8 @@ bindings_core_v8_files =
"core/v8/ExceptionState.h",
"core/v8/GeneratedCodeHelper.cpp",
"core/v8/GeneratedCodeHelper.h",
"core/v8/InitializeV8ExtrasBinding.cpp",
"core/v8/InitializeV8ExtrasBinding.h",
"core/v8/IDLDictionaryBase.cpp",
"core/v8/IDLDictionaryBase.h",
"core/v8/IDLTypes.h",
......@@ -189,6 +191,7 @@ bindings_unittest_files =
"core/v8/BindingSecurityTest.cpp",
"core/v8/DictionaryTest.cpp",
"core/v8/DOMWrapperWorldTest.cpp",
"core/v8/InitializeV8ExtrasBindingTest.cpp",
"core/v8/IDLTypesTest.cpp",
"core/v8/NativeValueTraitsImplTest.cpp",
"core/v8/NativeValueTraitsTest.cpp",
......@@ -209,6 +212,8 @@ bindings_unittest_files =
"core/v8/serialization/SerializedScriptValueTest.cpp",
"core/v8/serialization/SerializedScriptValueThreadedTest.cpp",
"core/v8/serialization/V8ScriptValueSerializerTest.cpp",
"core/v8/V8ExtrasTestUtils.cpp",
"core/v8/V8ExtrasTestUtils.h",
],
"abspath")
bindings_unittest_files += bindings_modules_v8_unittest_files
third_party/WebKit/Source/bindings/core/v8/InitializeV8ExtrasBinding.cpp 0 → 100644
View file @ 2f3fc53f
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "bindings/core/v8/InitializeV8ExtrasBinding.h"
#include <algorithm>
#include <iterator>
#include "bindings/core/v8/ScriptFunction.h"
#include "bindings/core/v8/ToV8ForCore.h"
#include "core/dom/ExecutionContext.h"
#include "core/frame/UseCounter.h"
#include "core/frame/WebFeature.h"
#include "platform/bindings/ToV8.h"
#include "platform/bindings/V8Binding.h"
#include "platform/bindings/V8ThrowException.h"
#include "v8/include/v8.h"
namespace blink {
namespace {
// This macro avoids duplicating the name and hence prevents typos.
#define WEB_FEATURE_ID_NAME_LOOKUP_ENTRY(id) \
{ #id, WebFeature::k##id }
struct WebFeatureIdNameLookupEntry {
const char* const name;
const WebFeature id;
};
// TODO(ricea): Replace with a more efficient data structure if the
// number of entries increases.
constexpr WebFeatureIdNameLookupEntry web_feature_id_name_lookup_table[] = {
WEB_FEATURE_ID_NAME_LOOKUP_ENTRY(ReadableStreamConstructor),
WEB_FEATURE_ID_NAME_LOOKUP_ENTRY(WritableStreamConstructor),
WEB_FEATURE_ID_NAME_LOOKUP_ENTRY(TransformStreamConstructor),
};
#undef WEB_FEATURE_ID_NAME_LOOKUP_ENTRY
class CountUseForBindings : public ScriptFunction {
public:
static v8::Local<v8::Function> CreateFunction(ScriptState* script_state) {
auto* self = new CountUseForBindings(script_state);
return self->BindToV8Function();
}
private:
explicit CountUseForBindings(ScriptState* script_state)
: ScriptFunction(script_state) {}
ScriptValue Call(ScriptValue value) override {
String string_id;
if (!value.ToString(string_id)) {
V8ThrowException::ThrowTypeError(value.GetIsolate(),
"countUse requires a string argument");
return ScriptValue();
}
const auto it =
std::find_if(std::begin(web_feature_id_name_lookup_table),
std::end(web_feature_id_name_lookup_table),
[&string_id](const WebFeatureIdNameLookupEntry& entry) {
return string_id == entry.name;
});
if (it == std::end(web_feature_id_name_lookup_table)) {
V8ThrowException::ThrowTypeError(value.GetIsolate(),
"unknown use counter");
return ScriptValue();
}
UseCounter::Count(ExecutionContext::From(GetScriptState()), it->id);
return ScriptValue::From(GetScriptState(), ToV8UndefinedGenerator());
}
};
} // namespace
void InitializeV8ExtrasBinding(ScriptState* script_state) {
v8::Local<v8::Object> binding =
script_state->GetContext()->GetExtrasBindingObject();
v8::Local<v8::Function> fn =
CountUseForBindings::CreateFunction(script_state);
v8::Local<v8::String> name =
V8AtomicString(script_state->GetIsolate(), "countUse");
binding->Set(script_state->GetContext(), name, fn).FromJust();
}
} // namespace blink
third_party/WebKit/Source/bindings/core/v8/InitializeV8ExtrasBinding.h 0 → 100644
View file @ 2f3fc53f
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef InitializeV8ExtrasBinding_h
#define InitializeV8ExtrasBinding_h
#include "core/CoreExport.h"
namespace blink {
class ScriptState;
// Add the Javascript function countUse() to the "binding" object that is
// exposed to the Javascript streams implementations.
//
// It must be called during initialisation of the V8 context.
//
// binding.countUse() takes a string and calls UseCounter::Count() on the
// matching ID. It only does anything the first time it is called in a
// particular execution context. The use of a string argument avoids duplicating
// the IDs in the JS files, but means that JS code should avoid calling it more
// than once to avoid unnecessary overhead. Only string IDs that this code
// specifically knows about will work.
//
// countUse() is not available during snapshot creation.
void CORE_EXPORT InitializeV8ExtrasBinding(ScriptState*);
} // namespace blink
#endif // InitializeV8ExtrasBinding_h
third_party/WebKit/Source/bindings/core/v8/InitializeV8ExtrasBindingTest.cpp 0 → 100644
View file @ 2f3fc53f
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "bindings/core/v8/InitializeV8ExtrasBinding.h"
#include "bindings/core/v8/V8BindingForTesting.h"
#include "bindings/core/v8/V8ExtrasTestUtils.h"
#include "core/dom/Document.h"
#include "core/frame/UseCounter.h"
#include "core/frame/WebFeature.h"
#include "platform/bindings/V8Binding.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "v8/include/v8.h"
namespace blink {
namespace {
// Add "binding" to the global object. Production code should never do this;
// it would be a huge security hole.
void AddExtrasBindingToGlobal(V8TestingScope* scope) {
auto context = scope->GetContext();
v8::Local<v8::Object> global = context->Global();
v8::Local<v8::Object> binding = context->GetExtrasBindingObject();
v8::Local<v8::String> key = V8AtomicString(scope->GetIsolate(), "binding");
global->Set(context, key, binding).FromJust();
}
TEST(InitializeV8ExtrasBindingTest, SupportedId) {
V8TestingScope scope;
InitializeV8ExtrasBinding(scope.GetScriptState());
AddExtrasBindingToGlobal(&scope);
ScriptValue rv = EvalWithPrintingError(
&scope, "binding.countUse('TransformStreamConstructor');");
EXPECT_TRUE(rv.IsUndefined());
EXPECT_TRUE(UseCounter::IsCounted(scope.GetDocument(),
WebFeature::kTransformStreamConstructor));
}
TEST(InitializeV8ExtrasBindingTest, UnsupportedId) {
V8TestingScope scope;
InitializeV8ExtrasBinding(scope.GetScriptState());
AddExtrasBindingToGlobal(&scope);
ScriptValue rv = EvalWithPrintingError(&scope,
"let result;"
"try {"
" binding.countUse('WindowEvent');"
" result = 'FAIL';"
"} catch (e) {"
" result = e.name;"
"}"
"result");
String result;
EXPECT_TRUE(rv.ToString(result));
EXPECT_EQ("TypeError", result);
}
} // namespace
} // namespace blink
third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
View file @ 2f3fc53f
......@@ -30,6 +30,7 @@
#include "bindings/core/v8/LocalWindowProxy.h"
#include "bindings/core/v8/InitializeV8ExtrasBinding.h"
#include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/ToV8ForCore.h"
#include "bindings/core/v8/V8BindingForCore.h"
......@@ -237,6 +238,8 @@ void LocalWindowProxy::CreateContext() {
script_state_ = ScriptState::Create(context, world_);
InitializeV8ExtrasBinding(script_state_.get());
DCHECK(lifecycle_ == Lifecycle::kContextIsUninitialized ||
lifecycle_ == Lifecycle::kGlobalObjectIsDetached);
lifecycle_ = Lifecycle::kContextIsInitialized;
......
third_party/WebKit/Source/bindings/core/v8/V8ExtrasTestUtils.cpp 0 → 100644
View file @ 2f3fc53f
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "bindings/core/v8/V8ExtrasTestUtils.h"
#include "bindings/core/v8/ScriptValue.h"
#include "bindings/core/v8/V8BindingForTesting.h"
#include "platform/bindings/V8Binding.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace blink {
TryCatchScope::TryCatchScope(v8::Isolate* isolate)
: isolate_(isolate), trycatch_(isolate) {}
TryCatchScope::~TryCatchScope() {
v8::MicrotasksScope::PerformCheckpoint(isolate_);
EXPECT_FALSE(trycatch_.HasCaught());
}
ScriptValue Eval(V8TestingScope* scope, const char* script_as_string) {
v8::Local<v8::String> source;
v8::Local<v8::Script> script;
v8::MicrotasksScope microtasks(scope->GetIsolate(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
// TODO(ricea): Can this actually fail? Should it be a DCHECK?
if (!v8::String::NewFromUtf8(scope->GetIsolate(), script_as_string,
v8::NewStringType::kNormal)
.ToLocal(&source)) {
ADD_FAILURE();
return ScriptValue();
}
if (!v8::Script::Compile(scope->GetContext(), source).ToLocal(&script)) {
ADD_FAILURE() << "Compilation fails";
return ScriptValue();
}
return ScriptValue(scope->GetScriptState(), script->Run(scope->GetContext()));
}
ScriptValue EvalWithPrintingError(V8TestingScope* scope, const char* script) {
v8::TryCatch block(scope->GetIsolate());
ScriptValue r = Eval(scope, script);
if (block.HasCaught()) {
ADD_FAILURE() << ToCoreString(
block.Exception()->ToString(scope->GetIsolate()))
.Utf8()
.data();
block.ReThrow();
}
return r;
}
} // namespace blink
third_party/WebKit/Source/bindings/core/v8/V8ExtrasTestUtils.h 0 → 100644
View file @ 2f3fc53f
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Useful utilities for testing V8 extras and streams.
#ifndef V8ExtrasTestUtils_h
#define V8ExtrasTestUtils_h
#include "v8/include/v8.h"
namespace blink {
class ScriptValue;
class V8TestingScope;
// If any exceptions are thrown while TryCatchScope is in scope, the test will
// fail.
class TryCatchScope {
public:
explicit TryCatchScope(v8::Isolate*);
~TryCatchScope();
private:
v8::Isolate* isolate_;
v8::TryCatch trycatch_;
};
// Evaluate the Javascript in "script" and return the result.
ScriptValue Eval(V8TestingScope*, const char* script);
// Evaluate the Javascript in "script" and return the result, failing the test
// if an exception is thrown.
ScriptValue EvalWithPrintingError(V8TestingScope*, const char* script);
} // namespace blink
#endif // V8ExtrasTestUtils_h
third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp
View file @ 2f3fc53f
......@@ -32,6 +32,7 @@
#include <memory>
#include "bindings/core/v8/InitializeV8ExtrasBinding.h"
#include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/ScriptSourceCode.h"
#include "bindings/core/v8/ScriptValue.h"
......@@ -181,6 +182,8 @@ bool WorkerOrWorkletScriptController::InitializeContextIfNeeded(
ScriptState::Scope scope(script_state_.get());
InitializeV8ExtrasBinding(script_state_.get());
// Associate the global proxy object, the global object and the worker
// instance (C++ object) as follows.
//
......
third_party/WebKit/Source/core/streams/ReadableStream.js
View file @ 2f3fc53f
......@@ -123,9 +123,19 @@
'Failed to execute \'pipeThrough\' on \'ReadableStream\': parameter ' +
'1\'s \'readable\' property is undefined.';
let useCounted = false;
class ReadableStream {
constructor(underlyingSource = {}, { size, highWaterMark = 1 } = {},
internalArgument = undefined) {
const internal =
internalArgument === createWithExternalControllerSentinel;
if (!useCounted && !internal) {
binding.countUse('ReadableStreamConstructor');
useCounted = true;
}
this[_readableStreamBits] = 0b0;
ReadableStreamSetState(this, STATE_READABLE);
this[_reader] = undefined;
......@@ -147,8 +157,7 @@
}
this[_controller] = new ReadableStreamDefaultController(
this, underlyingSource, size, highWaterMark,
internalArgument === createWithExternalControllerSentinel);
this, underlyingSource, size, highWaterMark, internal);
}
get locked() {
......
third_party/WebKit/Source/core/streams/ReadableStreamOperations.h
View file @ 2f3fc53f
......@@ -7,6 +7,7 @@
#include "bindings/core/v8/ScriptPromise.h"
#include "bindings/core/v8/ScriptValue.h"
#include "core/CoreExport.h"
#include "v8/include/v8.h"
namespace blink {
......
third_party/WebKit/Source/core/streams/ReadableStreamOperationsTest.cpp
View file @ 2f3fc53f
......@@ -9,6 +9,7 @@
#include "bindings/core/v8/ScriptValue.h"
#include "bindings/core/v8/V8BindingForCore.h"
#include "bindings/core/v8/V8BindingForTesting.h"
#include "bindings/core/v8/V8ExtrasTestUtils.h"
#include "bindings/core/v8/V8IteratorResultValue.h"
#include "core/dom/Document.h"
#include "core/streams/ReadableStreamDefaultControllerWrapper.h"
......@@ -115,52 +116,6 @@ class TestUnderlyingSource final : public UnderlyingSourceBase {
double DesiredSize() { return Controller()->DesiredSize(); }
};
class TryCatchScope {
public:
explicit TryCatchScope(v8::Isolate* isolate)
: isolate_(isolate), trycatch_(isolate) {}
~TryCatchScope() {
v8::MicrotasksScope::PerformCheckpoint(isolate_);
EXPECT_FALSE(trycatch_.HasCaught());
}
private:
v8::Isolate* isolate_;
v8::TryCatch trycatch_;
};
ScriptValue Eval(V8TestingScope* scope, const char* s) {
v8::Local<v8::String> source;
v8::Local<v8::Script> script;
v8::MicrotasksScope microtasks(scope->GetIsolate(),
v8::MicrotasksScope::kDoNotRunMicrotasks);
if (!v8::String::NewFromUtf8(scope->GetIsolate(), s,
v8::NewStringType::kNormal)
.ToLocal(&source)) {
ADD_FAILURE();
return ScriptValue();
}
if (!v8::Script::Compile(scope->GetContext(), source).ToLocal(&script)) {
ADD_FAILURE() << "Compilation fails";
return ScriptValue();
}
return ScriptValue(scope->GetScriptState(), script->Run(scope->GetContext()));
}
ScriptValue EvalWithPrintingError(V8TestingScope* scope, const char* s) {
v8::TryCatch block(scope->GetIsolate());
ScriptValue r = Eval(scope, s);
if (block.HasCaught()) {
ADD_FAILURE() << ToCoreString(
block.Exception()->ToString(scope->GetIsolate()))
.Utf8()
.data();
block.ReThrow();
}
return r;
}
TEST(ReadableStreamOperationsTest, IsReadableStream) {
V8TestingScope scope;
TryCatchScope try_catch_scope(scope.GetIsolate());
......
third_party/WebKit/Source/core/streams/TransformStream.js
View file @ 2f3fc53f
......@@ -56,9 +56,16 @@
const streamErrors = binding.streamErrors;
const errStreamTerminated = 'The transform stream has been terminated';
let useCounted = false;
class TransformStream {
constructor(transformer = {},
writableStrategy = {}, readableStrategy = {}) {
if (!useCounted) {
binding.countUse('TransformStreamConstructor');
useCounted = true;
}
// readable and writableType are extension points for future byte streams.
const readableType = transformer.readableType;
if (readableType !== undefined) {
......
third_party/WebKit/Source/core/streams/WritableStream.js
View file @ 2f3fc53f
......@@ -114,6 +114,8 @@
const verbClosed = 'closed';
const verbWrittenTo = 'written to';
let useCounted = false;
// Utility functions (not from the standard).
function createWriterLockReleasedError(verb) {
return new TypeError(errWriterLockReleasedPrefix + verb);
......@@ -136,6 +138,11 @@
class WritableStream {
constructor(underlyingSink = {}, strategy = {}) {
if (!useCounted) {
binding.countUse('WritableStreamConstructor');
useCounted = true;
}
InitializeWritableStream(this);
const type = underlyingSink.type;
const size = strategy.size;
......
third_party/WebKit/public/platform/web_feature.mojom
View file @ 2f3fc53f
......@@ -1888,6 +1888,9 @@ enum WebFeature {
kPPAPIURLRequestStreamToFile = 2396,
kPaymentHandler = 2397,
kPaymentRequestShowWithoutGesture = 2398,
kReadableStreamConstructor = 2399,
kWritableStreamConstructor = 2400,
kTransformStreamConstructor = 2401,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
......
tools/metrics/histograms/enums.xml
View file @ 2f3fc53f
......@@ -18042,6 +18042,9 @@ Called by update_net_error_codes.py.-->
<int value="2396" label="PPAPIURLRequestStreamToFile"/>
<int value="2397" label="PaymentHandler"/>
<int value="2398" label="PaymentRequestShowWithoutGesture"/>
<int value="2399" label="ReadableStreamConstructor"/>
<int value="2400" label="WritableStreamConstructor"/>
<int value="2401" label="TransformStreamConstructor"/>
</enum>
<enum name="FeedbackSource">
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment