Remove always-on flag --harmony-strict-legacy-accessor-builtins

It was shipped in Chrome 62.

Bug: v8:5070
Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng
Change-Id: I77119540411d1fe15691d40012cb96f4e2e45048
Reviewed-on: https://chromium-review.googlesource.com/776154Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49434}

src/bootstrapper.cc

@@ -4270,7 +4270,6 @@ EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_dynamic_import)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_import_meta)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_template_escapes)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_restrict_constructor_return)
-EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_strict_legacy_accessor_builtins)
 
 void InstallPublicSymbol(Factory* factory, Handle<Context> native_context,
                          const char* name, Handle<Symbol> value) {

src/builtins/builtins-object.cc

@@ -86,11 +86,8 @@ Object* ObjectDefineAccessor(Isolate* isolate, Handle<Object> object,
                              Handle<Object> name, Handle<Object> accessor) {
   // 1. Let O be ? ToObject(this value).
   Handle<JSReceiver> receiver;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, receiver,
-      FLAG_harmony_strict_legacy_accessor_builtins
-          ? Object::ToObject(isolate, object)
-          : Object::ConvertReceiver(isolate, object));
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
+                                     Object::ToObject(isolate, object));
   // 2. If IsCallable(getter) is false, throw a TypeError exception.
   if (!accessor->IsCallable()) {
     MessageTemplate::Template message =
@@ -116,10 +113,8 @@ Object* ObjectDefineAccessor(Isolate* isolate, Handle<Object> object,
   // 5. Perform ? DefinePropertyOrThrow(O, key, desc).
   // To preserve legacy behavior, we ignore errors silently rather than
   // throwing an exception.
-  Maybe<bool> success = JSReceiver::DefineOwnProperty(
-      isolate, receiver, name, &desc,
-      FLAG_harmony_strict_legacy_accessor_builtins ? kThrowOnError
-                                                   : kDontThrow);
+  Maybe<bool> success = JSReceiver::DefineOwnProperty(isolate, receiver, name,
+                                                      &desc, kThrowOnError);
   MAYBE_RETURN(success, isolate->heap()->exception());
   if (!success.FromJust()) {
     isolate->CountUsage(v8::Isolate::kDefineGetterOrSetterWouldThrow);
@@ -130,11 +125,8 @@ Object* ObjectDefineAccessor(Isolate* isolate, Handle<Object> object,
 
 Object* ObjectLookupAccessor(Isolate* isolate, Handle<Object> object,
                              Handle<Object> key, AccessorComponent component) {
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, object,
-      FLAG_harmony_strict_legacy_accessor_builtins
-          ? Object::ToObject(isolate, object)
-          : Object::ConvertReceiver(isolate, object));
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, object,
+                                     Object::ToObject(isolate, object));
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, key,
                                      Object::ToPropertyKey(isolate, key));
   bool success = false;

src/flag-definitions.h

@@ -210,8 +210,6 @@ DEFINE_IMPLICATION(harmony_import_meta, harmony_dynamic_import)
 
 // Features that are shipping (turned on by default, but internal flag remains).
 #define HARMONY_SHIPPING_BASE(V)                                        \
-  V(harmony_strict_legacy_accessor_builtins,                            \
-    "treat __defineGetter__ and related functions as strict")           \
   V(harmony_sharedarraybuffer, "harmony sharedarraybuffer")             \
   V(harmony_regexp_dotall, "harmony regexp dotAll flag")                \
   V(harmony_regexp_lookbehind, "harmony regexp lookbehind")             \

test/cctest/test-usecounters.cc

@@ -17,63 +17,6 @@ void MockUseCounterCallback(v8::Isolate* isolate,
   ++global_use_counts[feature];
 }
 
-TEST(DefineGetterSetterThrowUseCount) {
-  i::FLAG_harmony_strict_legacy_accessor_builtins = false;
-  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]);
-}
-
 TEST(AssigmentExpressionLHSIsCall) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope scope(isolate);
@@ -142,55 +85,6 @@ TEST(LabeledExpressionStatement) {
   CHECK_EQ(2, use_counts[v8::Isolate::kLabeledExpressionStatement]);
 }
 
-TEST(IndexAccessorUseCount) {
-  i::FLAG_harmony_strict_legacy_accessor_builtins = false;
-  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);
-
-  // Adding accessor to named property does not increment kIndexAccessor
-  CompileRun(
-      "var a = {};"
-      "function dummy(){}"
-      "Object.defineProperty(a, 'test', { get: dummy, set: dummy });");
-  CHECK_EQ(0, use_counts[v8::Isolate::kIndexAccessor]);
-
-  // Setting index to value does not increment kIndexAccessor
-  CompileRun(
-      "var a = {};"
-      "Object.defineProperty(a, '0', { value: 0 });");
-  CHECK_EQ(0, use_counts[v8::Isolate::kIndexAccessor]);
-
-  // Non-integer number index not increment kIndexAccessor
-  CompileRun(
-      "var a = {};"
-      "Object.defineProperty(a, '2.5', { value: 0 });");
-  CHECK_EQ(0, use_counts[v8::Isolate::kIndexAccessor]);
-
-  // Setting index accessor increments count
-  CompileRun(
-      "var a = {};"
-      "function dummy(){}"
-      "Object.defineProperty(a, '0', { get : dummy, set : dummy });");
-  CHECK_EQ(1, use_counts[v8::Isolate::kIndexAccessor]);
-
-  // Setting index accessor on array increments count
-  CompileRun(
-      "var a = [];"
-      "function dummy(){}"
-      "Object.defineProperty(a, '0', { get : dummy, set : dummy });");
-  CHECK_EQ(2, use_counts[v8::Isolate::kIndexAccessor]);
-
-  // __defineGetter__ increments count
-  CompileRun(
-      "var a = [];"
-      "function dummy(){}"
-      "a.__defineGetter__('0', dummy);");
-  CHECK_EQ(3, use_counts[v8::Isolate::kIndexAccessor]);
-}
 }  // namespace test_usecounters
 }  // namespace internal
 }  // namespace v8

test/mjsunit/global-accessors.js

@@ -26,16 +26,14 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 // Test accessors on the global object.
-//
-// Flags: --no-harmony-strict-legacy-accessor-builtins
 
 var x_ = 0;
 
-__defineSetter__('x', function(x) { x_ = x; });
-__defineGetter__('x', function() { return x_; });
+this.__defineSetter__('x', function(x) { x_ = x; });
+this.__defineGetter__('x', function() { return x_; });
 
-__defineSetter__('y', function(x) { });
-__defineGetter__('y', function() { return 7; });
+this.__defineSetter__('y', function(x) { });
+this.__defineGetter__('y', function() { return 7; });
 
 function f(a) {
   x = x + a;

test/mjsunit/harmony/global-accessors-strict.js

-// Copyright 2010 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-//       notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-//       copyright notice, this list of conditions and the following
-//       disclaimer in the documentation and/or other materials provided
-//       with the distribution.
-//     * Neither the name of Google Inc. nor the names of its
-//       contributors may be used to endorse or promote products derived
-//       from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// Test accessors on the global object.
-//
-// Flags: --harmony-strict-legacy-accessor-builtins
-
-var x_ = 0;
-
-this.__defineSetter__('x', function(x) { x_ = x; });
-this.__defineGetter__('x', function() { return x_; });
-
-this.__defineSetter__('y', function(x) { });
-this.__defineGetter__('y', function() { return 7; });
-
-function f(a) {
-  x = x + a;
-  return x;
-}
-
-function g(a) {
-  y = y + a;
-  return y;
-}
-
-assertEquals(1, f(1));
-assertEquals(3, f(2));
-
-assertEquals(7, g(1));
-assertEquals(7, g(2));

test/mozilla/mozilla.status

@@ -811,8 +811,8 @@
   # error message in debug mode.
   'js1_5/extensions/regress-336410-1': [FAIL_OK, ['mode == debug and arch == x64', NO_VARIANTS]],
 
-  # These tests fail when --harmony-strict-legacy-accessor-builtins
-  # is enabled.
+  # These tests fail due to __defineGetter__ & friends throwing
+  # for undefined receivers.
   'js1_5/extensions/regress-313500': [SKIP],
   'js1_5/extensions/regress-325269': [SKIP],
 

test/test262/test262.status

@@ -240,16 +240,6 @@
   'built-ins/TypedArrays/internals/DefineOwnProperty/conversion-operation-consistent-nan': [PASS, FAIL],
   'built-ins/TypedArrays/internals/Set/conversion-operation-consistent-nan': [PASS, FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=5070
-  'annexB/built-ins/Object/prototype/__defineGetter__/define-non-configurable': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__defineGetter__/define-non-extensible': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__defineGetter__/this-non-obj': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__defineSetter__/define-non-configurable': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__defineSetter__/define-non-extensible': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__defineSetter__/this-non-obj': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__lookupGetter__/this-non-obj': ['--harmony-strict-legacy-accessor-builtins'],
-  'annexB/built-ins/Object/prototype/__lookupSetter__/this-non-obj': ['--harmony-strict-legacy-accessor-builtins'],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=4451
   # https://github.com/tc39/ecma262/issues/753
   'annexB/language/eval-code/direct/global-block-decl-eval-global-existing-global-init': [FAIL],