From 19a28dcefd941f3a38c15695111827d7da74dbd2 Mon Sep 17 00:00:00 2001 From: machenbach Date: Mon, 11 May 2015 01:20:02 -0700 Subject: [PATCH] Revert of Add the concept of a V8 extras exports object (patchset #5 id:80001 of https://codereview.chromium.org/1128113006/) Reason for revert: [Sheriff] Causes gc stress failures: http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20gc%20stress/builds/2167 Original issue's description: > Add the concept of a V8 extras exports object > > Exposed to the extras as extrasExports (on the builtins object), on > which they can put things that should be accessible from C++. Exposed > to C++ through the V8 API as v8::Context::GetExtrasExportsObject(). > > Adding a test (in test-api.cc) required adding a simple extra, > test-extra.js, which we build into the standalone builds. > > R=yangguo@chromium.org, jochen@chromium.org > BUG= > > Committed: https://crrev.com/ad547cea05f3e02c67243b682e933fc53ac763d9 > Cr-Commit-Position: refs/heads/master@{#28317} TBR=jochen@chromium.org,yangguo@chromium.org,domenic@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/1127313005 Cr-Commit-Position: refs/heads/master@{#28332} --- build/standalone.gypi | 3 --- include/v8.h | 6 ------ src/api.cc | 8 -------- src/bootstrapper.cc | 16 ---------------- src/contexts.h | 4 +--- src/runtime/runtime-test.cc | 6 ------ src/runtime/runtime.h | 1 - test/cctest/test-api.cc | 18 ------------------ test/cctest/test-extra.js | 13 ------------- test/mjsunit/debug-script.js | 2 +- 10 files changed, 2 insertions(+), 75 deletions(-) delete mode 100644 test/cctest/test-extra.js diff --git a/build/standalone.gypi b/build/standalone.gypi index d2d14ebcd64..7c967206734 100644 --- a/build/standalone.gypi +++ b/build/standalone.gypi @@ -100,9 +100,6 @@ 'msan%': '<(msan)', 'tsan%': '<(tsan)', - # Add a simple extra solely for the purpose of the cctests - 'v8_extra_library_files': ['../test/cctest/test-extra.js'], - # .gyp files or targets should set v8_code to 1 if they build V8 specific # code, as opposed to external code. This variable is used to control such # things as the set of warnings to enable, and whether warnings are treated diff --git a/include/v8.h b/include/v8.h index 52270737596..910279b52e6 100644 --- a/include/v8.h +++ b/include/v8.h @@ -6370,12 +6370,6 @@ class V8_EXPORT Context { */ V8_INLINE Local GetEmbedderData(int index); - /** - * Gets the exports object used by V8 extras. Extra natives get a reference - * to this object and can use it to export functionality. - */ - Local GetExtrasExportsObject(); - /** * Sets the embedder data with the given index, growing the data as * needed. Note that index 0 currently has a special meaning for Chrome's diff --git a/src/api.cc b/src/api.cc index f1d23c4e927..4f16120df33 100644 --- a/src/api.cc +++ b/src/api.cc @@ -5492,14 +5492,6 @@ void Context::DetachGlobal() { } -Local Context::GetExtrasExportsObject() { - i::Handle context = Utils::OpenHandle(this); - i::Isolate* isolate = context->GetIsolate(); - i::Handle exports(context->extras_exports_object(), isolate); - return Utils::ToLocal(exports); -} - - void Context::AllowCodeGenerationFromStrings(bool allow) { i::Handle context = Utils::OpenHandle(this); i::Isolate* isolate = context->GetIsolate(); diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index eb0c396cc3e..f3dd682871b 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -201,7 +201,6 @@ class Genesis BASE_EMBEDDED { void InitializeGlobal(Handle global_object, Handle empty_function); void InitializeExperimentalGlobal(); - void InitializeExtrasExportsObject(); // Installs the contents of the native .js files on the global objects. // Used for creating a context from scratch. void InstallNativeFunctions(); @@ -1442,20 +1441,6 @@ void Genesis::InitializeExperimentalGlobal() { } -void Genesis::InitializeExtrasExportsObject() { - Handle exports = - factory()->NewJSObject(isolate()->object_function(), TENURED); - - native_context()->set_extras_exports_object(*exports); - - Handle builtins(native_context()->builtins()); - Handle exports_string = - factory()->InternalizeOneByteString(STATIC_CHAR_VECTOR("extrasExports")); - Runtime::SetObjectProperty(isolate(), builtins, exports_string, exports, - STRICT).Assert(); -} - - bool Genesis::CompileBuiltin(Isolate* isolate, int index) { Vector name = Natives::GetScriptName(index); Handle source_code = @@ -2984,7 +2969,6 @@ Genesis::Genesis(Isolate* isolate, // them after they have already been deserialized would also fail. if (!isolate->serializer_enabled()) { InitializeExperimentalGlobal(); - InitializeExtrasExportsObject(); if (!InstallExperimentalNatives()) return; if (!InstallExtraNatives()) return; } diff --git a/src/contexts.h b/src/contexts.h index 2aef4c381b0..2d04da29b3b 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -185,8 +185,7 @@ enum BindingFlags { V(MAP_ITERATOR_MAP_INDEX, Map, map_iterator_map) \ V(SET_ITERATOR_MAP_INDEX, Map, set_iterator_map) \ V(ARRAY_VALUES_ITERATOR_INDEX, JSFunction, array_values_iterator) \ - V(SCRIPT_CONTEXT_TABLE_INDEX, ScriptContextTable, script_context_table) \ - V(EXTRAS_EXPORTS_OBJECT_INDEX, JSObject, extras_exports_object) + V(SCRIPT_CONTEXT_TABLE_INDEX, ScriptContextTable, script_context_table) // A table of all script contexts. Every loaded top-level script with top-level @@ -423,7 +422,6 @@ class Context: public FixedArray { SCRIPT_CONTEXT_TABLE_INDEX, MAP_CACHE_INDEX, TO_LENGTH_FUN_INDEX, - EXTRAS_EXPORTS_OBJECT_INDEX, // Properties from here are treated as weak references by the full GC. // Scavenge treats them as strong references. diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 4c49d327b2b..ddf2e9d6b46 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -364,12 +364,6 @@ RUNTIME_FUNCTION(Runtime_NativeScriptsCount) { } -RUNTIME_FUNCTION(Runtime_NativeExtrasCount) { - DCHECK(args.length() == 0); - return Smi::FromInt(ExtraNatives::GetBuiltinsCount()); -} - - // Returns V8 version as a string. RUNTIME_FUNCTION(Runtime_GetV8Version) { HandleScope scope(isolate); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index fd92cb6f123..f87e993cc80 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -592,7 +592,6 @@ namespace internal { F(Abort, 1, 1) \ F(AbortJS, 1, 1) \ F(NativeScriptsCount, 0, 1) \ - F(NativeExtrasCount, 0, 1) \ F(GetV8Version, 0, 1) \ F(DisassembleFunction, 1, 1) \ F(TraceEnter, 0, 1) \ diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 43d2c744739..a434098cc0a 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -21029,21 +21029,3 @@ TEST(SealHandleScopeNested) { USE(obj); } } - - -TEST(ExtrasExportsObject) { - v8::Isolate* isolate = CcTest::isolate(); - v8::HandleScope handle_scope(isolate); - LocalContext env; - - // standalone.gypi ensures we include the test-extra.js file, which should - // add the testExtraShouldReturnFive export - v8::Local exports = env->GetExtrasExportsObject(); - - auto func = - exports->Get(v8_str("testExtraShouldReturnFive")).As(); - auto undefined = v8::Undefined(isolate); - auto result = func->Call(undefined, 0, {}).As(); - - CHECK(result->Value() == 5.0); -} diff --git a/test/cctest/test-extra.js b/test/cctest/test-extra.js deleted file mode 100644 index bc4eacc23da..00000000000 --- a/test/cctest/test-extra.js +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2015 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. - -(function () { - - 'use strict'; - - extrasExports.testExtraShouldReturnFive = function () { - return 5; - }; - -}); diff --git a/test/mjsunit/debug-script.js b/test/mjsunit/debug-script.js index 12a1f16d35f..af1eb454d65 100644 --- a/test/mjsunit/debug-script.js +++ b/test/mjsunit/debug-script.js @@ -66,7 +66,7 @@ for (i = 0; i < scripts.length; i++) { } // This has to be updated if the number of native scripts change. -assertEquals(%NativeScriptsCount() + %NativeExtrasCount(), named_native_count); +assertEquals(%NativeScriptsCount(), named_native_count); // Only the 'gc' extension is loaded. assertEquals(1, extension_count); // This script and mjsunit.js has been loaded. If using d8, d8 loads