Update JsBuiltIn register functions to use common logic

Author: jackhorton

lib/Runtime/Library/EngineInterfaceObject.cpp

@@ -268,11 +268,9 @@ namespace Js
     }
 
     /* static */
-    template <bool isConstructor>
-    ScriptFunction *EngineInterfaceObject::CreateLibraryCodeScriptFunction(ScriptFunction *scriptFunction, JavascriptString *displayName)
+    ScriptFunction *EngineInterfaceObject::CreateLibraryCodeScriptFunction(ScriptFunction *scriptFunction, JavascriptString *displayName, bool isConstructor, bool isJsBuiltIn, bool isPublic)
     {
         ScriptContext *scriptContext = scriptFunction->GetScriptContext();
-        scriptFunction->GetFunctionProxy()->SetIsPublicLibraryCode();
 
         // Use GetSz rather than GetString because we use wcsrchr below, which expects a null-terminated string
         // Callers can pass in a string like "get compare" or "Intl.Collator.prototype.resolvedOptions" -- only for the
@@ -310,6 +308,7 @@ namespace Js
             AssertMsg((scriptFunction->GetFunctionInfo()->GetAttributes() & FunctionInfo::Attributes::ErrorOnNew) == 0, "Why is the function not constructable by default?");
         }
 
+        // handle the name property AFTER handling isConstructor, because this can initialize the function's deferred type
         Var existingName = nullptr;
         if (JavascriptOperators::GetOwnProperty(scriptFunction, PropertyIds::name, &existingName, scriptContext, nullptr))
         {
@@ -334,6 +333,24 @@ namespace Js
             scriptFunction->SetPropertyWithAttributes(PropertyIds::name, funcName, PropertyConfigurable, nullptr);
         }
 
+        if (isJsBuiltIn)
+        {
+            scriptFunction->GetFunctionProxy()->SetIsJsBuiltInCode();
+
+            // This makes it so that the given scriptFunction can't reference/close over any outside variables,
+            // which is desirable for JsBuiltIns (though currently not for Intl)
+            scriptFunction->SetEnvironment(const_cast<FrameDisplay *>(&StrictNullFrameDisplay));
+
+            // TODO(jahorto): investigate force-inlining Intl code
+            AssertOrFailFast(scriptFunction->HasFunctionBody());
+            scriptFunction->GetFunctionBody()->SetJsBuiltInForceInline();
+        }
+
+        if (isPublic)
+        {
+            scriptFunction->GetFunctionProxy()->SetIsPublicLibraryCode();
+        }
+
         return scriptFunction;
     }
 
@@ -349,17 +366,15 @@ namespace Js
         ScriptFunction *func = ScriptFunction::UnsafeFromVar(args[1]);
         JavascriptString *methodName = JavascriptString::UnsafeFromVar(args[2]);
 
+        bool isConstructor = true;
         if (args.Info.Count == 4)
         {
             AssertOrFailFast(JavascriptBoolean::Is(args.Values[3]));
-            if (!JavascriptBoolean::UnsafeFromVar(args.Values[3])->GetValue())
-            {
-                return CreateLibraryCodeScriptFunction<false>(func, methodName);
-            }
+            isConstructor = JavascriptBoolean::UnsafeFromVar(args.Values[3])->GetValue();
         }
 
         // isConstructor = true is the default (when no 3rd arg is provided)
-        return CreateLibraryCodeScriptFunction<true>(func, methodName);
+        return CreateLibraryCodeScriptFunction(func, methodName, isConstructor, false /* isJsBuiltIn */, true /* isPublic */);
     }
 
     /*

lib/Runtime/Library/EngineInterfaceObject.h

@@ -88,8 +88,7 @@ namespace Js
 
         static bool __cdecl InitializeCommonNativeInterfaces(DynamicObject* engineInterface, DeferredTypeHandlerBase * typeHandler, DeferredInitializeMode mode);
 
-        template <bool isConstructor>
-        static ScriptFunction *CreateLibraryCodeScriptFunction(ScriptFunction *scriptFunction, JavascriptString *displayName);
+        static ScriptFunction *CreateLibraryCodeScriptFunction(ScriptFunction *scriptFunction, JavascriptString *displayName, bool isConstructor, bool isJsBuiltIn, bool isPublic);
 
         class EntryInfo
         {

lib/Runtime/Library/JsBuiltInEngineInterfaceExtensionObject.cpp

@@ -310,29 +310,16 @@ namespace Js
     {
         EngineInterfaceObject_CommonFunctionProlog(function, callInfo);
 
-        AssertOrFailFast(args.Info.Count >= 3 && JavascriptString::Is(args.Values[1]) && JavascriptFunction::Is(args.Values[2]));
+        AssertOrFailFast(args.Info.Count >= 3 && JavascriptString::Is(args.Values[1]) && ScriptFunction::Is(args.Values[2]));
 
         JavascriptLibrary * library = scriptContext->GetLibrary();
 
-        // retrieves arguments
-        JavascriptString* methodName = JavascriptString::FromVar(args.Values[1]);
-        JavascriptFunction* func = JavascriptFunction::FromVar(args.Values[2]);
-
-        func->GetFunctionProxy()->EnsureDeserialized();
+        JavascriptString* methodName = JavascriptString::UnsafeFromVar(args.Values[1]);
+        ScriptFunction* func = EngineInterfaceObject::CreateLibraryCodeScriptFunction(ScriptFunction::UnsafeFromVar(args.Values[2]), methodName, false /* isConstructor */, true /* isJsBuiltIn */, false /* isPublic */);
 
-        DynamicObject* chakraLibraryObject = GetPrototypeFromName(PropertyIds::__chakraLibrary, false, scriptContext);
         PropertyIds functionIdentifier = JavascriptOperators::GetPropertyId(methodName, scriptContext);
 
-        // Link the function to __chakraLibrary.
-        ScriptFunction* scriptFunction = library->CreateScriptFunction(func->GetFunctionProxy());
-        scriptFunction->GetFunctionProxy()->SetIsJsBuiltInCode();
-
-        Assert(scriptFunction->HasFunctionBody());
-        scriptFunction->GetFunctionBody()->SetJsBuiltInForceInline();
-
-        scriptFunction->SetPropertyWithAttributes(PropertyIds::name, methodName, PropertyConfigurable, nullptr);
-
-        library->AddMember(chakraLibraryObject, functionIdentifier, scriptFunction);
+        library->AddMember(library->GetChakraLib(), functionIdentifier, func);
 
         //Don't need to return anything
         return library->GetUndefined();
@@ -342,7 +329,7 @@ namespace Js
     {
         EngineInterfaceObject_CommonFunctionProlog(function, callInfo);
 
-        AssertOrFailFast(args.Info.Count >= 3 && JavascriptObject::Is(args.Values[1]) && JavascriptFunction::Is(args.Values[2]));
+        AssertOrFailFast(args.Info.Count >= 3 && JavascriptObject::Is(args.Values[1]) && ScriptFunction::Is(args.Values[2]));
 
         JavascriptLibrary * library = scriptContext->GetLibrary();
 
@@ -370,49 +357,33 @@ namespace Js
 
         BOOL staticMethod = JavascriptConversion::ToBoolean(staticMethodProperty, scriptContext);
         BOOL forceInline = JavascriptConversion::ToBoolean(forceInlineProperty, scriptContext);
+        Assert(forceInline);
 
-        JavascriptFunction* func = JavascriptFunction::FromVar(args.Values[2]);
-
-        // Set the function's display name, as the function we pass in argument are anonym.
-        func->GetFunctionProxy()->SetIsPublicLibraryCode();
-        func->GetFunctionProxy()->EnsureDeserialized()->SetDisplayName(methodName->GetString(), methodName->GetLength(), 0);
+        ScriptFunction *func = EngineInterfaceObject::CreateLibraryCodeScriptFunction(ScriptFunction::UnsafeFromVar(args.Values[2]), methodName, false /* isConstructor */, true /* isJsBuiltIn */, true /* isPublic */);
 
         DynamicObject* prototype = GetPrototypeFromName(JavascriptOperators::GetPropertyId(className, scriptContext), staticMethod, scriptContext);
-        PropertyIds functionIdentifier = methodName->BufferEquals(_u("Symbol.iterator"), 15)? PropertyIds::_symbolIterator :
-            JavascriptOperators::GetPropertyId(methodName, scriptContext);
-
-        // Link the function to the prototype.
-        ScriptFunction* scriptFunction = library->CreateScriptFunction(func->GetFunctionProxy());
-        scriptFunction->GetFunctionProxy()->SetIsJsBuiltInCode();
-
-        if (forceInline)
-        {
-            Assert(scriptFunction->HasFunctionBody());
-            scriptFunction->GetFunctionBody()->SetJsBuiltInForceInline();
-        }
-        scriptFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(argumentsCount), PropertyConfigurable, nullptr);
-
-        scriptFunction->SetConfigurable(PropertyIds::prototype, true);
-        scriptFunction->DeleteProperty(PropertyIds::prototype, Js::PropertyOperationFlags::PropertyOperation_None);
+        PropertyIds functionIdentifier = methodName->BufferEquals(_u("Symbol.iterator"), 15)
+            ? PropertyIds::_symbolIterator
+            : JavascriptOperators::GetPropertyId(methodName, scriptContext);
 
-        scriptFunction->SetPropertyWithAttributes(PropertyIds::name, methodName, PropertyConfigurable, nullptr);
+        func->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(argumentsCount), PropertyConfigurable, nullptr);
 
-        library->AddMember(prototype, functionIdentifier, scriptFunction);
+        library->AddMember(prototype, functionIdentifier, func);
 
-        RecordCommonNativeInterfaceBuiltIns(functionIdentifier, scriptContext, scriptFunction);
+        RecordCommonNativeInterfaceBuiltIns(functionIdentifier, scriptContext, func);
 
         if (!JavascriptOperators::IsUndefinedOrNull(aliasProperty))
         {
             JavascriptString * alias = JavascriptConversion::ToString(aliasProperty, scriptContext);
             // Cannot do a string to property id search here, Symbol.* have different hashing mechanism, so resort to this str compare
             PropertyIds aliasFunctionIdentifier = alias->BufferEquals(_u("Symbol.iterator"), 15) ? PropertyIds::_symbolIterator :
                 JavascriptOperators::GetPropertyId(alias, scriptContext);
-            library->AddMember(prototype, aliasFunctionIdentifier, scriptFunction);
+            library->AddMember(prototype, aliasFunctionIdentifier, func);
         }
 
         if (prototype == library->arrayPrototype)
         {
-            RecordDefaultIteratorFunctions(functionIdentifier, scriptContext, scriptFunction);
+            RecordDefaultIteratorFunctions(functionIdentifier, scriptContext, func);
         }
 
         //Don't need to return anything