Refactor CreateApiFunction

BUG=

Review-Url: https://codereview.chromium.org/2095953002
Cr-Commit-Position: refs/heads/master@{#37290}

src/accessors.cc

@@ -664,29 +664,10 @@ Handle<AccessorInfo> Accessors::ScriptEvalFromFunctionNameInfo(
 // Accessors::FunctionPrototype
 //
 
-static Handle<Object> GetFunctionPrototype(Isolate* 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);
-}
-
-
-MUST_USE_RESULT static MaybeHandle<Object> SetFunctionPrototype(
-    Isolate* isolate, Handle<JSFunction> function, Handle<Object> value) {
-  JSFunction::SetPrototype(function, value);
-  DCHECK(function->prototype() == *value);
-  return function;
-}
-
-
 MaybeHandle<Object> Accessors::FunctionSetPrototype(Handle<JSFunction> function,
                                                     Handle<Object> prototype) {
-  DCHECK(function->IsConstructor());
+  JSFunction::SetPrototype(function, prototype);
-  Isolate* isolate = function->GetIsolate();
+  return function;
-  return SetFunctionPrototype(isolate, function, prototype);
 }
 
 
@@ -697,7 +678,7 @@ void Accessors::FunctionPrototypeGetter(
   HandleScope scope(isolate);
   Handle<JSFunction> function =
       Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
-  Handle<Object> result = GetFunctionPrototype(isolate, function);
+  Handle<Object> result = JSFunction::GetPrototype(isolate, function);
   info.GetReturnValue().Set(Utils::ToLocal(result));
 }
 
@@ -711,9 +692,7 @@ void Accessors::FunctionPrototypeSetter(
   Handle<Object> value = Utils::OpenHandle(*val);
   Handle<JSFunction> object =
       Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
-  if (SetFunctionPrototype(isolate, object, value).is_null()) {
+  JSFunction::SetPrototype(object, value);
-    isolate->OptionalRescheduleException(false);
-  }
 }
 
 

src/api-natives.cc

@@ -371,43 +371,12 @@ MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
       return handle(JSFunction::cast(element), isolate);
     }
   }
+
   // Enter a new scope.  Recursion could otherwise create a lot of handles.
   HandleScope scope(isolate);
-  Handle<JSObject> prototype;
+
-  if (!data->remove_prototype()) {
+  auto function =
-    auto prototype_templ = handle(data->prototype_template(), isolate);
+      ApiNatives::CreateApiFunction(isolate, data, JS_API_OBJECT_TYPE);
-    if (prototype_templ->IsUndefined(isolate)) {
-      prototype = isolate->factory()->NewJSObject(isolate->object_function());
-    } else {
-      ASSIGN_RETURN_ON_EXCEPTION(
-          isolate, prototype,
-          InstantiateObject(isolate,
-                            Handle<ObjectTemplateInfo>::cast(prototype_templ),
-                            Handle<JSReceiver>(), data->hidden_prototype()),
-          JSFunction);
-    }
-    auto parent = handle(data->parent_template(), isolate);
-    if (!parent->IsUndefined(isolate)) {
-      Handle<JSFunction> parent_instance;
-      ASSIGN_RETURN_ON_EXCEPTION(
-          isolate, parent_instance,
-          InstantiateFunction(isolate,
-                              Handle<FunctionTemplateInfo>::cast(parent)),
-          JSFunction);
-      // TODO(dcarney): decide what to do here.
-      Handle<Object> parent_prototype;
-      ASSIGN_RETURN_ON_EXCEPTION(
-          isolate, parent_prototype,
-          JSObject::GetProperty(parent_instance,
-                                isolate->factory()->prototype_string()),
-          JSFunction);
-      MAYBE_RETURN(JSObject::SetPrototype(prototype, parent_prototype, false,
-                                          Object::THROW_ON_ERROR),
-                   MaybeHandle<JSFunction>());
-    }
-  }
-  auto function = ApiNatives::CreateApiFunction(
-      isolate, data, prototype, ApiNatives::JavaScriptObjectType);
   if (!name.is_null() && name->IsString()) {
     function->shared()->set_name(*name);
   }
@@ -533,20 +502,17 @@ void ApiNatives::AddNativeDataProperty(Isolate* isolate,
   array.add(isolate, property);
 }
 
-
 Handle<JSFunction> ApiNatives::CreateApiFunction(
-    Isolate* isolate, Handle<FunctionTemplateInfo> obj,
+    Isolate* isolate, Handle<FunctionTemplateInfo> obj, InstanceType type) {
-    Handle<Object> prototype, ApiInstanceType instance_type) {
   Handle<SharedFunctionInfo> shared =
       FunctionTemplateInfo::GetOrCreateSharedFunctionInfo(isolate, obj);
+  DCHECK(shared->IsApiFunction());
   Handle<JSFunction> result =
       isolate->factory()->NewFunctionFromSharedFunctionInfo(
           shared, isolate->native_context());
 
   if (obj->remove_prototype()) {
     result->set_map(*isolate->sloppy_function_without_prototype_map());
-    DCHECK(prototype.is_null());
-    DCHECK(result->shared()->IsApiFunction());
     DCHECK(!result->has_initial_map());
     DCHECK(!result->has_prototype());
     DCHECK(!result->IsConstructor());
@@ -556,78 +522,81 @@ Handle<JSFunction> ApiNatives::CreateApiFunction(
   // Down from here is only valid for API functions that can be used as a
   // constructor (don't set the "remove prototype" flag).
 
-  if (obj->read_only_prototype()) {
+  // Set up function.prototype.
-    result->set_map(*isolate->sloppy_function_with_readonly_prototype_map());
+  Handle<JSObject> prototype;
-  }
+  auto prototype_templ = handle(obj->prototype_template(), isolate);
-
+  if (type != JS_API_OBJECT_TYPE || prototype_templ->IsUndefined(isolate)) {
-  if (prototype->IsTheHole(isolate)) {
     prototype = isolate->factory()->NewFunctionPrototype(result);
   } else {
+    prototype = internal::InstantiateObject(
+                    isolate, Handle<ObjectTemplateInfo>::cast(prototype_templ),
+                    Handle<JSReceiver>(), obj->hidden_prototype())
+                    .ToHandleChecked();
+
     JSObject::AddProperty(Handle<JSObject>::cast(prototype),
                           isolate->factory()->constructor_string(), result,
                           DONT_ENUM);
   }
+  // Set up function.prototype.__proto__.
+  auto parent = handle(obj->parent_template(), isolate);
+  if (!parent->IsUndefined(isolate)) {
+    Handle<JSFunction> parent_instance =
+        internal::InstantiateFunction(
+            isolate, Handle<FunctionTemplateInfo>::cast(parent))
+            .ToHandleChecked();
+    Handle<Object> parent_prototype =
+        JSFunction::GetPrototype(isolate, parent_instance);
+    CHECK(JSObject::SetPrototype(prototype, parent_prototype, false,
+                                 Object::THROW_ON_ERROR)
+              .IsJust());
+  }
 
+  if (obj->read_only_prototype()) {
+    result->set_map(*isolate->sloppy_function_with_readonly_prototype_map());
+  }
+
+  // Set up the function's initial map.
   int internal_field_count = 0;
   if (!obj->instance_template()->IsUndefined(isolate)) {
-    Handle<ObjectTemplateInfo> instance_template = Handle<ObjectTemplateInfo>(
+    Handle<ObjectTemplateInfo> instance_template(
         ObjectTemplateInfo::cast(obj->instance_template()));
     internal_field_count =
         Smi::cast(instance_template->internal_field_count())->value();
   }
 
-  // TODO(svenpanne) Kill ApiInstanceType and refactor things by generalizing
-  // JSObject::GetHeaderSize.
   int instance_size = kPointerSize * internal_field_count;
-  InstanceType type;
+  switch (type) {
-  switch (instance_type) {
+    case JS_API_OBJECT_TYPE:
-    case JavaScriptObjectType:
+      if (obj->needs_access_check() ||
-      if (!obj->needs_access_check() &&
+          !obj->named_property_handler()->IsUndefined(isolate) ||
-          obj->named_property_handler()->IsUndefined(isolate) &&
+          !obj->indexed_property_handler()->IsUndefined(isolate)) {
-          obj->indexed_property_handler()->IsUndefined(isolate)) {
-        type = JS_API_OBJECT_TYPE;
-      } else {
         type = JS_SPECIAL_API_OBJECT_TYPE;
       }
       instance_size += JSObject::kHeaderSize;
       break;
-    case GlobalObjectType:
+    case JS_GLOBAL_OBJECT_TYPE:
-      type = JS_GLOBAL_OBJECT_TYPE;
       instance_size += JSGlobalObject::kSize;
       break;
-    case GlobalProxyType:
+    case JS_GLOBAL_PROXY_TYPE:
-      type = JS_GLOBAL_PROXY_TYPE;
       instance_size += JSGlobalProxy::kSize;
       break;
     default:
       UNREACHABLE();
-      type = JS_OBJECT_TYPE;  // Keep the compiler happy.
       break;
   }
 
   Handle<Map> map =
       isolate->factory()->NewMap(type, instance_size, FAST_HOLEY_SMI_ELEMENTS);
-  JSFunction::SetInitialMap(result, map, Handle<JSObject>::cast(prototype));
+  JSFunction::SetInitialMap(result, map, prototype);
 
-  // Mark as undetectable if needed.
+  if (obj->undetectable()) map->set_is_undetectable();
-  if (obj->undetectable()) {
+  if (obj->needs_access_check()) map->set_is_access_check_needed(true);
-    map->set_is_undetectable();
-  }
-
-  // Mark as needs_access_check if needed.
-  if (obj->needs_access_check()) {
-    map->set_is_access_check_needed(true);
-  }
-
-  // Set interceptor information in the map.
   if (!obj->named_property_handler()->IsUndefined(isolate)) {
     map->set_has_named_interceptor();
   }
   if (!obj->indexed_property_handler()->IsUndefined(isolate)) {
     map->set_has_indexed_interceptor();
   }
-
-  // Mark instance as callable in the map.
   if (!obj->instance_call_handler()->IsUndefined(isolate)) {
     map->set_is_callable();
     map->set_is_constructor(true);

src/api-natives.h

@@ -6,6 +6,7 @@
 #define V8_API_NATIVES_H_
 
 #include "src/handles.h"
+#include "src/objects.h"
 #include "src/property-details.h"
 
 namespace v8 {
@@ -26,16 +27,9 @@ class ApiNatives {
       Handle<ObjectTemplateInfo> data,
       Handle<JSReceiver> new_target = Handle<JSReceiver>());
 
-  enum ApiInstanceType {
-    JavaScriptObjectType,
-    GlobalObjectType,
-    GlobalProxyType
-  };
-
   static Handle<JSFunction> CreateApiFunction(Isolate* isolate,
                                               Handle<FunctionTemplateInfo> obj,
-                                              Handle<Object> prototype,
+                                              InstanceType type);
-                                              ApiInstanceType instance_type);
 
   static void AddDataProperty(Isolate* isolate, Handle<TemplateInfo> info,
                               Handle<Name> name, Handle<Object> value,

src/bootstrapper.cc

@@ -1012,23 +1012,13 @@ Handle<JSGlobalObject> Genesis::CreateNewGlobals(
   if (js_global_object_template.is_null()) {
     Handle<String> name = Handle<String>(heap()->empty_string());
     Handle<Code> code = isolate()->builtins()->Illegal();
-    Handle<JSObject> prototype =
-        factory()->NewFunctionPrototype(isolate()->object_function());
     js_global_object_function = factory()->NewFunction(
-        name, code, prototype, JS_GLOBAL_OBJECT_TYPE, JSGlobalObject::kSize);
+        name, code, JS_GLOBAL_OBJECT_TYPE, JSGlobalObject::kSize);
-#ifdef DEBUG
-    LookupIterator it(prototype, factory()->constructor_string(),
-                      LookupIterator::OWN_SKIP_INTERCEPTOR);
-    Handle<Object> value = Object::GetProperty(&it).ToHandleChecked();
-    DCHECK(it.IsFound());
-    DCHECK_EQ(*isolate()->object_function(), *value);
-#endif
   } else {
     Handle<FunctionTemplateInfo> js_global_object_constructor(
         FunctionTemplateInfo::cast(js_global_object_template->constructor()));
     js_global_object_function = ApiNatives::CreateApiFunction(
-        isolate(), js_global_object_constructor, factory()->the_hole_value(),
+        isolate(), js_global_object_constructor, JS_GLOBAL_OBJECT_TYPE);
-        ApiNatives::GlobalObjectType);
   }
 
   js_global_object_function->initial_map()->set_is_prototype_map(true);
@@ -1049,8 +1039,7 @@ Handle<JSGlobalObject> Genesis::CreateNewGlobals(
     Handle<FunctionTemplateInfo> global_constructor(
             FunctionTemplateInfo::cast(data->constructor()));
     global_proxy_function = ApiNatives::CreateApiFunction(
-        isolate(), global_constructor, factory()->the_hole_value(),
+        isolate(), global_constructor, JS_GLOBAL_PROXY_TYPE);
-        ApiNatives::GlobalProxyType);
   }
   Handle<String> global_name = factory()->global_string();
   global_proxy_function->shared()->set_instance_class_name(*global_name);
@@ -2923,10 +2912,10 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
     // objects, that JavaScript code may not access.
     Handle<JSFunction> opaque_reference_fun = factory()->NewFunction(
         factory()->empty_string(), isolate()->builtins()->Illegal(),
-        isolate()->initial_object_prototype(), JS_VALUE_TYPE, JSValue::kSize);
+        JS_VALUE_TYPE, JSValue::kSize);
     Handle<JSObject> prototype =
         factory()->NewJSObject(isolate()->object_function(), TENURED);
-    Accessors::FunctionSetPrototype(opaque_reference_fun, prototype).Assert();
+    JSFunction::SetPrototype(opaque_reference_fun, prototype);
     native_context()->set_opaque_reference_function(*opaque_reference_fun);
   }
 
@@ -2952,11 +2941,6 @@ bool Genesis::InstallNatives(GlobalContextType context_type) {
 
   if (!CallUtilsFunction(isolate(), "PostNatives")) return false;
 
-  auto template_instantiations_cache = UnseededNumberDictionary::New(
-      isolate(), ApiNatives::kInitialFunctionCacheSize);
-  native_context()->set_template_instantiations_cache(
-      *template_instantiations_cache);
-
   // Store the map for the %ObjectPrototype% after the natives has been compiled
   // and the Object function has been set up.
   Handle<JSFunction> object_function(native_context()->object_function());
@@ -3873,6 +3857,14 @@ Genesis::Genesis(Isolate* isolate,
     CreateStrictModeFunctionMaps(empty_function);
     CreateIteratorMaps(empty_function);
     CreateAsyncFunctionMaps(empty_function);
+
+    // Set up the template instantiations cache before creating the globals,
+    // since they may want to use the cache.
+    auto template_instantiations_cache = UnseededNumberDictionary::New(
+        isolate, ApiNatives::kInitialFunctionCacheSize);
+    native_context()->set_template_instantiations_cache(
+        *template_instantiations_cache);
+
     Handle<JSGlobalObject> global_object =
         CreateNewGlobals(global_proxy_template, global_proxy);
     HookUpGlobalProxy(global_object, global_proxy);

src/factory.cc

@@ -1276,36 +1276,23 @@ Handle<JSFunction> Factory::NewFunctionWithoutPrototype(Handle<String> name,
 }
 
 
-Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
-                                        Handle<Object> prototype,
-                                        bool is_strict) {
-  Handle<Map> map = is_strict ? isolate()->strict_function_map()
-                              : isolate()->sloppy_function_map();
-  Handle<JSFunction> result = NewFunction(map, name, code);
-  result->set_prototype_or_initial_map(*prototype);
-  return result;
-}
-
-
 Handle<JSFunction> Factory::NewFunction(Handle<String> name, Handle<Code> code,
                                         Handle<Object> prototype,
                                         InstanceType type, int instance_size,
                                         bool is_strict) {
   // Allocate the function
-  Handle<JSFunction> function = NewFunction(name, code, prototype, is_strict);
+  Handle<Map> map = is_strict ? isolate()->strict_function_map()
+                              : isolate()->sloppy_function_map();
+  Handle<JSFunction> function = NewFunction(map, name, code);
+  DCHECK(prototype->IsTheHole(isolate()) || prototype->IsNull(isolate()) ||
+         prototype->IsJSReceiver());
+  function->set_prototype_or_initial_map(*prototype);
 
+  // TODO(verwaest): Do we need to eagerly allocate the map here?
   ElementsKind elements_kind =
       type == JS_ARRAY_TYPE ? FAST_SMI_ELEMENTS : FAST_HOLEY_SMI_ELEMENTS;
   Handle<Map> initial_map = NewMap(type, instance_size, elements_kind);
-  // TODO(littledan): Why do we have this is_generator test when
+  prototype = JSFunction::GetPrototype(isolate(), function);
-  // NewFunctionPrototype already handles finding an appropriately
-  // shared prototype?
-  if (!function->shared()->is_resumable()) {
-    if (prototype->IsTheHole(isolate())) {
-      prototype = NewFunctionPrototype(function);
-    }
-  }
-
   JSFunction::SetInitialMap(function, initial_map,
                             Handle<JSReceiver>::cast(prototype));
 

src/factory.h

@@ -506,9 +506,6 @@ class Factory final {
 
   Handle<JSGlobalProxy> NewUninitializedJSGlobalProxy();
 
-  Handle<JSFunction> NewFunction(Handle<String> name, Handle<Code> code,
-                                 Handle<Object> prototype,
-                                 bool is_strict = false);
   Handle<JSFunction> NewFunction(Handle<String> name);
   Handle<JSFunction> NewFunctionWithoutPrototype(Handle<String> name,
                                                  Handle<Code> code,

src/objects.cc

@@ -5357,6 +5357,17 @@ MaybeHandle<String> JSBoundFunction::GetName(Isolate* isolate,
   return factory->NewConsString(prefix, Handle<String>::cast(target_name));
 }
 
+// static
+Handle<Object> JSFunction::GetPrototype(Isolate* isolate,
+                                        Handle<JSFunction> function) {
+  DCHECK(function->IsConstructor() || function->shared()->is_generator());
+  if (!function->has_prototype()) {
+    Handle<Object> proto = isolate->factory()->NewFunctionPrototype(function);
+    JSFunction::SetPrototype(function, proto);
+  }
+  return handle(function->prototype(), isolate);
+}
+
 // static
 Handle<Object> JSFunction::GetName(Isolate* isolate,
                                    Handle<JSFunction> function) {

src/objects.h

@@ -7514,9 +7514,11 @@ class JSFunction: public JSObject {
   inline Context* native_context();
 
   static Handle<Object> GetName(Isolate* isolate, Handle<JSFunction> function);
-  static MaybeHandle<Smi> GetLength(Isolate* isolate,
+  static MUST_USE_RESULT MaybeHandle<Smi> GetLength(
-                                    Handle<JSFunction> function);
+      Isolate* isolate, Handle<JSFunction> function);
   static Handle<Context> GetFunctionRealm(Handle<JSFunction> function);
+  static Handle<Object> GetPrototype(Isolate* isolate,
+                                     Handle<JSFunction> function);
 
   // [code]: The generated code object for this function.  Executed
   // when the function is invoked, e.g. foo() or new foo(). See