Commit b2ce1fa2 authored by bakkot's avatar bakkot Committed by Commit bot

add use counters for __defineGetter__ failing

We deviate from spec in that, in our implementation, __defineGetter__ on non-
configurable properties returns false instead of throwing a TypeError. This commit
adds a use counter to track how often we would be throwing an error we currently
avoid, to determine if we can change to align with spec or if the spec is not
implementable.

BUG=v8:5070

Review-Url: https://codereview.chromium.org/2089533002
Cr-Commit-Position: refs/heads/master@{#37259}
parent 44ca8723
......@@ -5700,6 +5700,7 @@ class V8_EXPORT Isolate {
kRegExpPrototypeOldFlagGetter = 31,
kDecimalWithLeadingZeroInStrictMode = 32,
kLegacyDateParser = 33,
kDefineGetterOrSetterWouldThrow = 34,
// If you add new values here, you'll also need to update Chromium's:
// UseCounter.h, V8PerIsolateData.cpp, histograms.xml
......
......@@ -1729,6 +1729,9 @@ Object* ObjectDefineAccessor(Isolate* isolate, Handle<Object> object,
Maybe<bool> success = JSReceiver::DefineOwnProperty(
isolate, receiver, name, &desc, Object::DONT_THROW);
MAYBE_RETURN(success, isolate->heap()->exception());
if (!success.FromJust()) {
isolate->CountUsage(v8::Isolate::kDefineGetterOrSetterWouldThrow);
}
// 6. Return undefined.
return isolate->heap()->undefined_value();
}
......
......@@ -157,6 +157,7 @@ v8_executable("cctest") {
"test-unboxed-doubles.cc",
"test-unique.cc",
"test-unscopables-hidden-prototype.cc",
"test-usecounters.cc",
"test-utils.cc",
"test-version.cc",
"test-weakmaps.cc",
......
......@@ -192,6 +192,7 @@
'test-unboxed-doubles.cc',
'test-unique.cc',
'test-unscopables-hidden-prototype.cc',
'test-usecounters.cc',
'test-utils.cc',
'test-version.cc',
'test-weakmaps.cc',
......
// Copyright 2016 the V8 project 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 "src/v8.h"
#include "test/cctest/cctest.h"
namespace {
int* global_use_counts = NULL;
void MockUseCounterCallback(v8::Isolate* isolate,
v8::Isolate::UseCounterFeature feature) {
++global_use_counts[feature];
}
}
TEST(DefineGetterSetterThrowUseCount) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
LocalContext env;
int use_counts[v8::Isolate::kUseCounterFeatureCount] = {};
global_use_counts = use_counts;
CcTest::isolate()->SetUseCounterCallback(MockUseCounterCallback);
// __defineGetter__ and __defineSetter__ do not increment
// kDefineGetterOrSetterWouldThrow on success
CompileRun(
"var a = {};"
"Object.defineProperty(a, 'b', { value: 0, configurable: true });"
"a.__defineGetter__('b', ()=>{});");
CHECK_EQ(0, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
CompileRun(
"var a = {};"
"Object.defineProperty(a, 'b', { value: 0, configurable: true });"
"a.__defineSetter__('b', ()=>{});");
CHECK_EQ(0, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
// __defineGetter__ and __defineSetter__ do not increment
// kDefineGetterOrSetterWouldThrow on other errors
v8::Local<v8::Value> resultProxyThrow = CompileRun(
"var exception;"
"try {"
"var a = new Proxy({}, { defineProperty: ()=>{throw new Error;} });"
"a.__defineGetter__('b', ()=>{});"
"} catch (e) { exception = e; }"
"exception");
CHECK_EQ(0, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
CHECK(resultProxyThrow->IsObject());
resultProxyThrow = CompileRun(
"var exception;"
"try {"
"var a = new Proxy({}, { defineProperty: ()=>{throw new Error;} });"
"a.__defineSetter__('b', ()=>{});"
"} catch (e) { exception = e; }"
"exception");
CHECK_EQ(0, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
CHECK(resultProxyThrow->IsObject());
// __defineGetter__ and __defineSetter__ increment
// kDefineGetterOrSetterWouldThrow when they would throw per spec (B.2.2.2)
CompileRun(
"var a = {};"
"Object.defineProperty(a, 'b', { value: 0, configurable: false });"
"a.__defineGetter__('b', ()=>{});");
CHECK_EQ(1, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
CompileRun(
"var a = {};"
"Object.defineProperty(a, 'b', { value: 0, configurable: false });"
"a.__defineSetter__('b', ()=>{});");
CHECK_EQ(2, use_counts[v8::Isolate::kDefineGetterOrSetterWouldThrow]);
}
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