Fix accessors to use holder instead of this

src/accessors.cc

 // Copyright 2012 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 "src/accessors.h"
 #include "src/api.h"
 #include "src/compiler.h"
 #include "src/contexts.h"
 #include "src/deoptimizer.h"
 #include "src/execution.h"
 #include "src/factory.h"
 #include "src/frames-inl.h"
 #include "src/isolate.h"
 #include "src/list-inl.h"
 #include "src/property-details.h"
 #include "src/prototype.h"
 
 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,
     AccessorGetterCallback getter,
     AccessorSetterCallback setter,
     PropertyAttributes attributes) {
   Factory* factory = isolate->factory();
   Handle<ExecutableAccessorInfo> info = factory->NewExecutableAccessorInfo();
   info->set_property_attributes(attributes);
   info->set_all_can_read(false);
@@ skipping 121 matching lines @@
   return value;
 }
 
 
 void Accessors::ArrayLengthGetter(
     v8::Local<v8::String> name,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   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)));
 }
 
 
 void Accessors::ArrayLengthSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
@@ skipping 53 matching lines @@
 //
 // Accessors::StringLength
 //
 
 void Accessors::StringLengthGetter(
     v8::Local<v8::String> name,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   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)));
 }
 
 
 void Accessors::StringLengthSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> value,
     const v8::PropertyCallbackInfo<void>& info) {
   UNREACHABLE();
 }
@@ skipping 555 matching lines @@
                       &ScriptEvalFromFunctionNameSetter,
                       attributes);
 }
 
 
 //
 // Accessors::FunctionPrototype
 //
 
 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);
   }
   return Handle<Object>(function->prototype(), isolate);
 }
 
 
 static Handle<Object> SetFunctionPrototype(Isolate* isolate,
                                            Handle<JSObject> receiver,
@@ skipping 45 matching lines @@
   Isolate* isolate = function->GetIsolate();
   return SetFunctionPrototype(isolate, function, prototype);
 }
 
 
 void Accessors::FunctionPrototypeGetter(
     v8::Local<v8::String> name,
     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));
 }
 
 
 void Accessors::FunctionPrototypeSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
@@ skipping 18 matching lines @@
 //
 // Accessors::FunctionLength
 //
 
 
 void Accessors::FunctionLengthGetter(
     v8::Local<v8::String> name,
     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);
   info.GetReturnValue().Set(Utils::ToLocal(result));
 }
 
 
 void Accessors::FunctionLengthSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
@@ skipping 15 matching lines @@
 //
 // Accessors::FunctionName
 //
 
 
 void Accessors::FunctionNameGetter(
     v8::Local<v8::String> name,
     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));
 }
 
 
 void Accessors::FunctionNameSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
   // Do nothing.
 }
@@ skipping 113 matching lines @@
 Handle<Object> Accessors::FunctionGetArguments(Handle<JSFunction> function) {
   return GetFunctionArguments(function->GetIsolate(), function);
 }
 
 
 void Accessors::FunctionArgumentsGetter(
     v8::Local<v8::String> name,
     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));
 }
 
 
 void Accessors::FunctionArgumentsSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
   // Do nothing.
 }
@@ skipping 119 matching lines @@
   }
   return Handle<JSFunction>(caller);
 }
 
 
 void Accessors::FunctionCallerGetter(
     v8::Local<v8::String> name,
     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));
 }
 
 
 void Accessors::FunctionCallerSetter(
     v8::Local<v8::String> name,
     v8::Local<v8::Value> val,
     const v8::PropertyCallbackInfo<void>& info) {
   // Do nothing.
@@ skipping 69 matching lines @@
   info->set_data(Smi::FromInt(index));
   Handle<Object> getter = v8::FromCData(isolate, &ModuleGetExport);
   Handle<Object> setter = v8::FromCData(isolate, &ModuleSetExport);
   info->set_getter(*getter);
   if (!(attributes & ReadOnly)) info->set_setter(*setter);
   return info;
 }
 
 
 } }  // namespace v8::internal