Fix accessors to use holder instead of this

src/accessors.cc

Index: src/accessors.cc
diff --git a/src/accessors.cc b/src/accessors.cc
index 182f0f63420e26e6c20cfbb3239a3e0f5200082d..882304a29cf88432ddf79bbb27d305f166327825 100644
--- a/src/accessors.cc
+++ b/src/accessors.cc
@@ -21,16 +21,6 @@ namespace v8 {
 namespace internal {
 
 
-// We have a slight impedance mismatch between the external API and the way we
-// use callbacks internally: Externally, callbacks can only be used with
-// v8::Object, but internally we even have callbacks on entities which are
-// higher in the hierarchy, so we can only return i::Object here, not
-// i::JSObject.
-Handle<Object> GetThisFrom(const v8::PropertyCallbackInfo<v8::Value>& info) {
-  return Utils::OpenHandle(*v8::Local<v8::Value>(info.This()));
-}
-
-
 Handle<AccessorInfo> Accessors::MakeAccessor(
     Isolate* isolate,
     Handle<String> name,
@@ -172,15 +162,8 @@ void Accessors::ArrayLengthGetter(
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   DisallowHeapAllocation no_allocation;
   HandleScope scope(isolate);
-  Object* object = *GetThisFrom(info);
-  // Traverse the prototype chain until we reach an array.
-  JSArray* holder = FindInstanceOf<JSArray>(isolate, object);
-  Object* result;
-  if (holder != NULL) {
-    result = holder->length();
-  } else {
-    result = Smi::FromInt(0);
-  }
+  JSArray* holder = JSArray::cast(*Utils::OpenHandle(*info.Holder()));
+  Object* result = holder->length();
   info.GetReturnValue().Set(Utils::ToLocal(Handle<Object>(result, isolate)));
 }
 
@@ -254,16 +237,20 @@ void Accessors::StringLengthGetter(
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   DisallowHeapAllocation no_allocation;
   HandleScope scope(isolate);
-  Object* value = *GetThisFrom(info);
-  Object* result;
-  if (value->IsJSValue()) value = JSValue::cast(value)->value();
-  if (value->IsString()) {
-    result = Smi::FromInt(String::cast(value)->length());
-  } else {
-    // If object is not a string we return 0 to be compatible with WebKit.
-    // Note: Firefox returns the length of ToString(object).
-    result = Smi::FromInt(0);
+
+  // We have a slight impedance mismatch between the external API and the way we
+  // use callbacks internally: Externally, callbacks can only be used with
+  // v8::Object, but internally we have callbacks on entities which are higher
+  // in the hierarchy, in this case for String values.
+
+  Object* value = *Utils::OpenHandle(*v8::Local<v8::Value>(info.This()));
+  if (!value->IsString()) {
+    // Either a String wrapper or a JSObject with a StringWrapper in its
+    // prototype chain.
+    ASSERT(value->IsJSObject());

[Toon Verwaest, 2014/07/23 16:56:28]

As the failing test indicates, this is wrong since you can put this in the
prototype chain of any other primitive value. Just remove this ASSERT and update
the comment.

[arv (Not doing code reviews), 2014/07/23 17:03:39]

I just saw that too. Removing the assert and updated the comment.

+    value = JSValue::cast(*Utils::OpenHandle(*info.Holder()))->value();
   }
+  Object* result = Smi::FromInt(String::cast(value)->length());
   info.GetReturnValue().Set(Utils::ToLocal(Handle<Object>(result, isolate)));
 }
 
@@ -839,21 +826,7 @@ Handle<AccessorInfo> Accessors::ScriptEvalFromFunctionNameInfo(
 //
 
 static Handle<Object> GetFunctionPrototype(Isolate* isolate,
-                                           Handle<Object> receiver) {
-  Handle<JSFunction> function;
-  {
-    DisallowHeapAllocation no_allocation;
-    JSFunction* function_raw = FindInstanceOf<JSFunction>(isolate, *receiver);
-    if (function_raw == NULL) return isolate->factory()->undefined_value();
-    while (!function_raw->should_have_prototype()) {
-      PrototypeIterator iter(isolate, function_raw);
-      function_raw = FindInstanceOf<JSFunction>(isolate, iter.GetCurrent());
-      // There has to be one because we hit the getter.
-      ASSERT(function_raw != NULL);
-    }
-    function = Handle<JSFunction>(function_raw, isolate);
-  }
-
+                                           Handle<JSFunction> function) {
   if (!function->has_prototype()) {
     Handle<Object> proto = isolate->factory()->NewFunctionPrototype(function);
     JSFunction::SetPrototype(function, proto);
@@ -919,8 +892,9 @@ void Accessors::FunctionPrototypeGetter(
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
-  Handle<Object> object = GetThisFrom(info);
-  Handle<Object> result = GetFunctionPrototype(isolate, object);
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
+  Handle<Object> result = GetFunctionPrototype(isolate, function);
   info.GetReturnValue().Set(Utils::ToLocal(result));
 }
 
@@ -959,29 +933,20 @@ void Accessors::FunctionLengthGetter(
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
-  Handle<Object> object = GetThisFrom(info);
-  MaybeHandle<JSFunction> maybe_function;
-
-  {
-    DisallowHeapAllocation no_allocation;
-    JSFunction* function = FindInstanceOf<JSFunction>(isolate, *object);
-    if (function != NULL) maybe_function = Handle<JSFunction>(function);
-  }
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
 
   int length = 0;
-  Handle<JSFunction> function;
-  if (maybe_function.ToHandle(&function)) {
-    if (function->shared()->is_compiled()) {
+  if (function->shared()->is_compiled()) {
+    length = function->shared()->length();
+  } else {
+    // If the function isn't compiled yet, the length is not computed
+    // correctly yet. Compile it now and return the right length.
+    if (Compiler::EnsureCompiled(function, KEEP_EXCEPTION)) {
       length = function->shared()->length();
-    } else {
-      // If the function isn't compiled yet, the length is not computed
-      // correctly yet. Compile it now and return the right length.
-      if (Compiler::EnsureCompiled(function, KEEP_EXCEPTION)) {
-        length = function->shared()->length();
-      }
-      if (isolate->has_pending_exception()) {
-        isolate->OptionalRescheduleException(false);
-      }
+    }
+    if (isolate->has_pending_exception()) {
+      isolate->OptionalRescheduleException(false);
     }
   }
   Handle<Object> result(Smi::FromInt(length), isolate);
@@ -1017,22 +982,9 @@ void Accessors::FunctionNameGetter(
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
-  Handle<Object> object = GetThisFrom(info);
-  MaybeHandle<JSFunction> maybe_function;
-
-  {
-    DisallowHeapAllocation no_allocation;
-    JSFunction* function = FindInstanceOf<JSFunction>(isolate, *object);
-    if (function != NULL) maybe_function = Handle<JSFunction>(function);
-  }
-
-  Handle<JSFunction> function;
-  Handle<Object> result;
-  if (maybe_function.ToHandle(&function)) {
-    result = Handle<Object>(function->shared()->name(), isolate);
-  } else {
-    result = isolate->factory()->undefined_value();
-  }
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
+  Handle<Object> result = Handle<Object>(function->shared()->name(), isolate);

[Toon Verwaest, 2014/07/23 16:56:28]

You can just write this as Handle<Object> result(function->shared()->name(),
isolate);

[arv (Not doing code reviews), 2014/07/23 17:03:39]

Done.

   info.GetReturnValue().Set(Utils::ToLocal(result));
 }
 
@@ -1166,22 +1118,9 @@ void Accessors::FunctionArgumentsGetter(
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
-  Handle<Object> object = GetThisFrom(info);
-  MaybeHandle<JSFunction> maybe_function;
-
-  {
-    DisallowHeapAllocation no_allocation;
-    JSFunction* function = FindInstanceOf<JSFunction>(isolate, *object);
-    if (function != NULL) maybe_function = Handle<JSFunction>(function);
-  }
-
-  Handle<JSFunction> function;
-  Handle<Object> result;
-  if (maybe_function.ToHandle(&function)) {
-    result = GetFunctionArguments(isolate, function);
-  } else {
-    result = isolate->factory()->undefined_value();
-  }
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
+  Handle<Object> result = GetFunctionArguments(isolate, function);
   info.GetReturnValue().Set(Utils::ToLocal(result));
 }
 
@@ -1321,26 +1260,16 @@ void Accessors::FunctionCallerGetter(
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
-  Handle<Object> object = GetThisFrom(info);
-  MaybeHandle<JSFunction> maybe_function;
-  {
-    DisallowHeapAllocation no_allocation;
-    JSFunction* function = FindInstanceOf<JSFunction>(isolate, *object);
-    if (function != NULL) maybe_function = Handle<JSFunction>(function);
-  }
-  Handle<JSFunction> function;
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
   Handle<Object> result;
-  if (maybe_function.ToHandle(&function)) {
-    MaybeHandle<JSFunction> maybe_caller;
-    maybe_caller = FindCaller(isolate, function);
-    Handle<JSFunction> caller;
-    if (maybe_caller.ToHandle(&caller)) {
-      result = caller;
-    } else {
-      result = isolate->factory()->null_value();
-    }
+  MaybeHandle<JSFunction> maybe_caller;
+  maybe_caller = FindCaller(isolate, function);
+  Handle<JSFunction> caller;
+  if (maybe_caller.ToHandle(&caller)) {
+    result = caller;
   } else {
-    result = isolate->factory()->undefined_value();
+    result = isolate->factory()->null_value();
   }
   info.GetReturnValue().Set(Utils::ToLocal(result));
 }