From 396236bfa0ba78ad5ca97229e0f6817f573c8202 Mon Sep 17 00:00:00 2001 From: yangguo Date: Tue, 12 May 2015 06:03:59 -0700 Subject: [PATCH] Revert of Provide accessor for object internal properties that doesn't require debugger to be active (patchset #3 id:40001 of https://codereview.chromium.org/1126103006/) Reason for revert: GC mole issues: https://chromegw.corp.google.com/i/client.v8/builders/V8%20Linux%20-%20gcmole/builds/1950/steps/GCMole%20ia32/logs/stdio Original issue's description: > Provide accessor for object internal properties that doesn't require debugger to be active > > Some of the DevTools' clients need to inspect JS objects without enabling debugger. This CL allows to inspect object's internal properties without enabling debugger and instantiating debug context. > > Note that now debug context can be created lazily if v8::Debug::GetDebugContext is called when there is no debug listener. This is fragile and has already resulted in some subtle error. I'm going to fix that in a separate CL. > > BUG=chromium:481845 > LOG=Y > > Committed: https://crrev.com/bdeb0de88c8cf5f2c78f261b45314138f525110d > Cr-Commit-Position: refs/heads/master@{#28362} TBR=pfeldman@chromium.org,kozyatinskiy@chromium.org,yurys@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=chromium:481845 Review URL: https://codereview.chromium.org/1133243002 Cr-Commit-Position: refs/heads/master@{#28365} --- include/v8-debug.h | 10 --- src/api.cc | 23 +++---- src/bootstrapper.cc | 1 - src/contexts.h | 2 - src/mirror-debugger.js | 55 ++++++++++++++-- src/objects.cc | 14 ---- src/objects.h | 1 - src/runtime/runtime-debug.cc | 150 ------------------------------------------- src/runtime/runtime.h | 4 -- 9 files changed, 60 insertions(+), 200 deletions(-) diff --git a/include/v8-debug.h b/include/v8-debug.h index d2683d0..e316934 100644 --- a/include/v8-debug.h +++ b/include/v8-debug.h @@ -260,16 +260,6 @@ class V8_EXPORT Debug { * unexpectedly used. LiveEdit is enabled by default. */ static void SetLiveEditEnabled(Isolate* isolate, bool enable); - - /** - * Returns array of internal properties specific to the value type. Result has - * the following - * format: [, ,...,, ]. Result array will be - * allocated in the current - * context. - */ - static MaybeLocal GetInternalProperties(Isolate* isolate, - Local value); }; diff --git a/src/api.cc b/src/api.cc index 39cc7e2..200dc5f 100644 --- a/src/api.cc +++ b/src/api.cc @@ -6090,7 +6090,16 @@ Local Array::CloneElementAt(uint32_t index) { bool Value::IsPromise() const { auto self = Utils::OpenHandle(this); - return i::Object::IsPromise(self); + if (!self->IsJSObject()) return false; + auto js_object = i::Handle::cast(self); + // Promises can't have access checks. + if (js_object->map()->is_access_check_needed()) return false; + auto isolate = js_object->GetIsolate(); + // TODO(dcarney): this should just be read from the symbol registry so as not + // to be context dependent. + auto key = isolate->promise_status(); + // Shouldn't be possible to throw here. + return i::JSObject::HasRealNamedProperty(js_object, key).FromJust(); } @@ -7385,18 +7394,6 @@ void Debug::SetLiveEditEnabled(Isolate* isolate, bool enable) { } -MaybeLocal Debug::GetInternalProperties(Isolate* v8_isolate, - Local value) { - i::Isolate* isolate = reinterpret_cast(v8_isolate); - ENTER_V8(isolate); - i::Handle val = Utils::OpenHandle(*value); - i::Handle result; - if (!i::Runtime::GetInternalProperties(isolate, val).ToHandle(&result)) - return MaybeLocal(); - return Utils::ToLocal(result); -} - - Handle CpuProfileNode::GetFunctionName() const { i::Isolate* isolate = i::Isolate::Current(); const i::ProfileNode* node = reinterpret_cast(this); diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index c612461..f3dd682 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -1611,7 +1611,6 @@ void Genesis::InstallNativeFunctions() { to_complete_property_descriptor); INSTALL_NATIVE(Symbol, "$promiseStatus", promise_status); - INSTALL_NATIVE(Symbol, "$promiseValue", promise_value); INSTALL_NATIVE(JSFunction, "$promiseCreate", promise_create); INSTALL_NATIVE(JSFunction, "$promiseResolve", promise_resolve); INSTALL_NATIVE(JSFunction, "$promiseReject", promise_reject); diff --git a/src/contexts.h b/src/contexts.h index e1ea906..2d04da2 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -157,7 +157,6 @@ enum BindingFlags { V(ERROR_MESSAGE_FOR_CODE_GEN_FROM_STRINGS_INDEX, Object, \ error_message_for_code_gen_from_strings) \ V(PROMISE_STATUS_INDEX, Symbol, promise_status) \ - V(PROMISE_VALUE_INDEX, Symbol, promise_value) \ V(PROMISE_CREATE_INDEX, JSFunction, promise_create) \ V(PROMISE_RESOLVE_INDEX, JSFunction, promise_resolve) \ V(PROMISE_REJECT_INDEX, JSFunction, promise_reject) \ @@ -394,7 +393,6 @@ class Context: public FixedArray { RUN_MICROTASKS_INDEX, ENQUEUE_MICROTASK_INDEX, PROMISE_STATUS_INDEX, - PROMISE_VALUE_INDEX, PROMISE_CREATE_INDEX, PROMISE_RESOLVE_INDEX, PROMISE_REJECT_INDEX, diff --git a/src/mirror-debugger.js b/src/mirror-debugger.js index dbdc68e..9ea23d7 100644 --- a/src/mirror-debugger.js +++ b/src/mirror-debugger.js @@ -904,12 +904,57 @@ ObjectMirror.prototype.toText = function() { * @return {Array} array (possibly empty) of InternalProperty instances */ ObjectMirror.GetInternalProperties = function(value) { - var properties = %DebugGetInternalProperties(value); - var result = []; - for (var i = 0; i < properties.length; i += 2) { - result.push(new InternalPropertyMirror(properties[i], properties[i + 1])); + if (IS_STRING_WRAPPER(value) || IS_NUMBER_WRAPPER(value) || + IS_BOOLEAN_WRAPPER(value)) { + var primitiveValue = %_ValueOf(value); + return [new InternalPropertyMirror("[[PrimitiveValue]]", primitiveValue)]; + } else if (IS_FUNCTION(value)) { + var bindings = %BoundFunctionGetBindings(value); + var result = []; + if (bindings && IS_ARRAY(bindings)) { + result.push(new InternalPropertyMirror("[[TargetFunction]]", + bindings[0])); + result.push(new InternalPropertyMirror("[[BoundThis]]", bindings[1])); + var boundArgs = []; + for (var i = 2; i < bindings.length; i++) { + boundArgs.push(bindings[i]); + } + result.push(new InternalPropertyMirror("[[BoundArgs]]", boundArgs)); + } + return result; + } else if (IS_MAP_ITERATOR(value) || IS_SET_ITERATOR(value)) { + var details = IS_MAP_ITERATOR(value) ? %MapIteratorDetails(value) + : %SetIteratorDetails(value); + var kind; + switch (details[2]) { + case 1: kind = "keys"; break; + case 2: kind = "values"; break; + case 3: kind = "entries"; break; + } + var result = [ + new InternalPropertyMirror("[[IteratorHasMore]]", details[0]), + new InternalPropertyMirror("[[IteratorIndex]]", details[1]) + ]; + if (kind) { + result.push(new InternalPropertyMirror("[[IteratorKind]]", kind)); + } + return result; + } else if (IS_GENERATOR(value)) { + return [ + new InternalPropertyMirror("[[GeneratorStatus]]", + GeneratorGetStatus_(value)), + new InternalPropertyMirror("[[GeneratorFunction]]", + %GeneratorGetFunction(value)), + new InternalPropertyMirror("[[GeneratorReceiver]]", + %GeneratorGetReceiver(value)) + ]; + } else if (ObjectIsPromise(value)) { + return [ + new InternalPropertyMirror("[[PromiseStatus]]", PromiseGetStatus_(value)), + new InternalPropertyMirror("[[PromiseValue]]", PromiseGetValue_(value)) + ]; } - return result; + return []; } diff --git a/src/objects.cc b/src/objects.cc index ba7eebb..9a5353f 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -112,20 +112,6 @@ bool Object::IsCallable() const { } -bool Object::IsPromise(Handle object) { - if (!object->IsJSObject()) return false; - auto js_object = Handle::cast(object); - // Promises can't have access checks. - if (js_object->map()->is_access_check_needed()) return false; - auto isolate = js_object->GetIsolate(); - // TODO(dcarney): this should just be read from the symbol registry so as not - // to be context dependent. - auto key = isolate->promise_status(); - // Shouldn't be possible to throw here. - return JSObject::HasRealNamedProperty(js_object, key).FromJust(); -} - - MaybeHandle Object::GetProperty(LookupIterator* it) { for (; it->IsFound(); it->Next()) { switch (it->state()) { diff --git a/src/objects.h b/src/objects.h index 705730f..eb978b9 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1056,7 +1056,6 @@ class Object { INLINE(bool IsOrderedHashSet() const); INLINE(bool IsOrderedHashMap() const); bool IsCallable() const; - static bool IsPromise(Handle object); // Oddball testing. INLINE(bool IsUndefined() const); diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index dd472c6..9696b76 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -101,156 +101,6 @@ static Handle DebugGetProperty(LookupIterator* it, } -static Handle DebugGetProperty(Handle object, - Handle name) { - LookupIterator it(object, name); - return DebugGetProperty(&it); -} - - -template -static MaybeHandle GetIteratorInternalProperties( - Isolate* isolate, Handle object) { - Factory* factory = isolate->factory(); - Handle iterator = Handle::cast(object); - RUNTIME_ASSERT_HANDLIFIED(iterator->kind()->IsSmi(), JSArray); - const char* kind = NULL; - switch (Smi::cast(iterator->kind())->value()) { - case IteratorType::kKindKeys: - kind = "keys"; - break; - case IteratorType::kKindValues: - kind = "values"; - break; - case IteratorType::kKindEntries: - kind = "entries"; - break; - default: - RUNTIME_ASSERT_HANDLIFIED(false, JSArray); - } - - Handle result = factory->NewFixedArray(2 * 3); - result->set(0, *factory->NewStringFromAsciiChecked("[[IteratorHasMore]]")); - result->set(1, isolate->heap()->ToBoolean(iterator->HasMore())); - - result->set(2, *factory->NewStringFromAsciiChecked("[[IteratorIndex]]")); - result->set(3, iterator->index()); - - result->set(4, *factory->NewStringFromAsciiChecked("[[IteratorKind]]")); - result->set(5, *factory->NewStringFromAsciiChecked(kind)); - return factory->NewJSArrayWithElements(result); -} - - -MaybeHandle Runtime::GetInternalProperties(Isolate* isolate, - Handle object) { - Factory* factory = isolate->factory(); - if (object->IsJSFunction()) { - Handle function = Handle::cast(object); - if (function->shared()->bound()) { - RUNTIME_ASSERT_HANDLIFIED(function->function_bindings()->IsFixedArray(), - JSArray); - - Handle bindings(function->function_bindings()); - - Handle result = factory->NewFixedArray(2 * 3); - result->set(0, *factory->NewStringFromAsciiChecked("[[TargetFunction]]")); - result->set(1, bindings->get(JSFunction::kBoundFunctionIndex)); - - result->set(2, *factory->NewStringFromAsciiChecked("[[BoundThis]]")); - result->set(3, bindings->get(JSFunction::kBoundThisIndex)); - - Handle arguments = factory->NewFixedArray( - bindings->length() - JSFunction::kBoundArgumentsStartIndex); - bindings->CopyTo( - JSFunction::kBoundArgumentsStartIndex, *arguments, 0, - bindings->length() - JSFunction::kBoundArgumentsStartIndex); - result->set(4, *factory->NewStringFromAsciiChecked("[[BoundArgs]]")); - result->set(5, *factory->NewJSArrayWithElements(arguments)); - return factory->NewJSArrayWithElements(result); - } - } else if (object->IsJSMapIterator()) { - Handle iterator = Handle::cast(object); - return GetIteratorInternalProperties(isolate, iterator); - } else if (object->IsJSSetIterator()) { - Handle iterator = Handle::cast(object); - return GetIteratorInternalProperties(isolate, iterator); - } else if (object->IsJSGeneratorObject()) { - Handle generator = - Handle::cast(object); - - const char* status = "suspended"; - if (generator->is_closed()) { - status = "closed"; - } else if (generator->is_executing()) { - status = "running"; - } else { - DCHECK(generator->is_suspended()); - } - - Handle result = factory->NewFixedArray(2 * 3); - result->set(0, *factory->NewStringFromAsciiChecked("[[GeneratorStatus]]")); - result->set(1, *factory->NewStringFromAsciiChecked(status)); - - result->set(2, - *factory->NewStringFromAsciiChecked("[[GeneratorFunction]]")); - result->set(3, generator->function()); - - result->set(4, - *factory->NewStringFromAsciiChecked("[[GeneratorReceiver]]")); - result->set(5, generator->receiver()); - return factory->NewJSArrayWithElements(result); - } else if (Object::IsPromise(object)) { - Handle promise = Handle::cast(object); - - Handle status_obj = - DebugGetProperty(promise, isolate->promise_status()); - RUNTIME_ASSERT_HANDLIFIED(status_obj->IsSmi(), JSArray); - const char* status = "rejected"; - int status_val = Handle::cast(status_obj)->value(); - switch (status_val) { - case +1: - status = "resolved"; - break; - case 0: - status = "pending"; - break; - default: - DCHECK_EQ(-1, status_val); - } - - Handle result = factory->NewFixedArray(2 * 2); - result->set(0, *factory->NewStringFromAsciiChecked("[[PromiseStatus]]")); - result->set(1, *factory->NewStringFromAsciiChecked(status)); - - Handle value_obj = - DebugGetProperty(promise, isolate->promise_value()); - result->set(2, *factory->NewStringFromAsciiChecked("[[PromiseValue]]")); - result->set(3, *value_obj); - return factory->NewJSArrayWithElements(result); - } else if (object->IsJSValue()) { - Handle js_value = Handle::cast(object); - - Handle result = factory->NewFixedArray(2); - result->set(0, *factory->NewStringFromAsciiChecked("[[PrimitiveValue]]")); - result->set(1, js_value->value()); - return factory->NewJSArrayWithElements(result); - } - return factory->NewJSArray(0); -} - - -RUNTIME_FUNCTION(Runtime_DebugGetInternalProperties) { - HandleScope scope(isolate); - DCHECK(args.length() == 1); - CONVERT_ARG_HANDLE_CHECKED(Object, obj, 0); - Handle result; - ASSIGN_RETURN_FAILURE_ON_EXCEPTION( - isolate, result, Runtime::GetInternalProperties(isolate, obj)); - return *result; -} - - // Get debugger related details for an object property, in the following format: // 0: Property value // 1: Property details diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 809ce66..ff1d759 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -134,7 +134,6 @@ namespace internal { F(DebugBreak, 0, 1) \ F(SetDebugEventListener, 2, 1) \ F(ScheduleBreak, 0, 1) \ - F(DebugGetInternalProperties, 1, 1) \ F(DebugGetPropertyDetails, 2, 1) \ F(DebugGetProperty, 2, 1) \ F(DebugPropertyTypeFromDetails, 1, 1) \ @@ -855,9 +854,6 @@ class Runtime : public AllStatic { Handle key, Handle value); static bool WeakCollectionDelete(Handle weak_collection, Handle key); - - static MaybeHandle GetInternalProperties(Isolate* isolate, - Handle); }; -- 2.7.4