[JSC] Drop translate phase in module loader

utatane.tea@gmail.com · utatane.tea@gmail.com · commit 86fe7b05167a · 2016-12-08T01:02:13.000Z
https://bugs.webkit.org/show_bug.cgi?id=164861 Reviewed by Saam Barati. Source/JavaScriptCore: Originally, this "translate" phase was introduced to the module loader. However, recent rework discussion[1] starts dropping this phase. And this "translate" phase is meaningless in the browser side module loader since this phase originally mimics the node.js's translation hook (like, transpiling CoffeeScript source to JavaScript). This "translate" phase is not necessary for the exposed HTML5 <script type="module"> tag right now. Once the module loader pipeline is redefined and specified, we need to update the current loader anyway. So dropping "translate" phase right now is OK. This a bit simplifies the current module loader pipeline. [1]: whatwg/loader#147 * builtins/ModuleLoaderPrototype.js: (newRegistryEntry): (fulfillFetch): (requestFetch): (requestInstantiate): (provide): (fulfillTranslate): Deleted. (requestTranslate): Deleted. * bytecode/BytecodeIntrinsicRegistry.cpp: (JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry): * jsc.cpp: * runtime/JSGlobalObject.cpp: * runtime/JSGlobalObject.h: * runtime/JSModuleLoader.cpp: (JSC::JSModuleLoader::translate): Deleted. * runtime/JSModuleLoader.h: * runtime/ModuleLoaderPrototype.cpp: (JSC::moduleLoaderPrototypeInstantiate): (JSC::moduleLoaderPrototypeTranslate): Deleted. Source/WebCore: * bindings/js/JSDOMWindowBase.cpp: * bindings/js/JSWorkerGlobalScopeBase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@209500 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,45 @@
+2016-12-07  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop translate phase in module loader
+        https://bugs.webkit.org/show_bug.cgi?id=164861
+
+        Reviewed by Saam Barati.
+
+        Originally, this "translate" phase was introduced to the module loader.
+        However, recent rework discussion[1] starts dropping this phase.
+        And this "translate" phase is meaningless in the browser side module loader
+        since this phase originally mimics the node.js's translation hook (like,
+        transpiling CoffeeScript source to JavaScript).
+
+        This "translate" phase is not necessary for the exposed HTML5
+        <script type="module"> tag right now. Once the module loader pipeline is
+        redefined and specified, we need to update the current loader anyway.
+        So dropping "translate" phase right now is OK.
+
+        This a bit simplifies the current module loader pipeline.
+
+        [1]: https://github.com/whatwg/loader/issues/147
+
+        * builtins/ModuleLoaderPrototype.js:
+        (newRegistryEntry):
+        (fulfillFetch):
+        (requestFetch):
+        (requestInstantiate):
+        (provide):
+        (fulfillTranslate): Deleted.
+        (requestTranslate): Deleted.
+        * bytecode/BytecodeIntrinsicRegistry.cpp:
+        (JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry):
+        * jsc.cpp:
+        * runtime/JSGlobalObject.cpp:
+        * runtime/JSGlobalObject.h:
+        * runtime/JSModuleLoader.cpp:
+        (JSC::JSModuleLoader::translate): Deleted.
+        * runtime/JSModuleLoader.h:
+        * runtime/ModuleLoaderPrototype.cpp:
+        (JSC::moduleLoaderPrototypeInstantiate):
+        (JSC::moduleLoaderPrototypeTranslate): Deleted.
+
 2016-12-07  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Add ability to distinguish if a Script was parsed as a module
diff --git a/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js b/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js
@@ -30,8 +30,7 @@
 // Currently, there are 4 hooks.
 //    1. Loader.resolve
 //    2. Loader.fetch
-//    3. Loader.translate
-//    4. Loader.instantiate
+//    3. Loader.instantiate
 
 function setStateToMax(entry, newState)
 {
@@ -54,36 +53,29 @@ function newRegistryEntry(key)
     //     a. If the status is Fetch and there is no entry.fetch promise, the entry is ready to fetch.
     //     b. If the status is Fetch and there is the entry.fetch promise, the entry is just fetching the resource.
     //
-    // 2. Translate
-    //     Ready to translate (or now translating) the raw fetched resource to the ECMAScript source code.
-    //     We can insert the hook that translates the resources e.g. transpilers.
-    //     a. If the status is Translate and there is no entry.translate promise, the entry is ready to translate.
-    //     b. If the status is Translate and there is the entry.translate promise, the entry is just translating
-    //        the payload to the source code.
-    //
-    // 3. Instantiate (AnalyzeModule)
-    //     Ready to instantiate (or now instantiating) the module record from the fetched (and translated)
+    // 2. Instantiate (AnalyzeModule)
+    //     Ready to instantiate (or now instantiating) the module record from the fetched
     //     source code.
     //     Typically, we parse the module code, extract the dependencies and binding information.
     //     a. If the status is Instantiate and there is no entry.instantiate promise, the entry is ready to instantiate.
-    //     b. If the status is Instantiate and there is the entry.translate promise, the entry is just instantiating
+    //     b. If the status is Instantiate and there is the entry.fetch promise, the entry is just instantiating
     //        the module record.
     //
-    // 4. Satisfy
+    // 3. Satisfy
     //     Ready to request the dependent modules (or now requesting & resolving).
     //     Without this state, the current draft causes infinite recursion when there is circular dependency.
     //     a. If the status is Satisfy and there is no entry.satisfy promise, the entry is ready to resolve the dependencies.
     //     b. If the status is Satisfy and there is the entry.satisfy promise, the entry is just resolving
     //        the dependencies.
     //
-    // 5. Link
+    // 4. Link
     //     Ready to link the module with the other modules.
     //     Linking means that the module imports and exports the bindings from/to the other modules.
     //
-    // 6. Ready
+    // 5. Ready
     //     The module is linked, so the module is ready to be executed.
     //
-    // Each registry entry has the 4 promises; "fetch", "translate", "instantiate" and "satisfy".
+    // Each registry entry has the 4 promises; "fetch", "instantiate" and "satisfy".
     // They are assigned when starting the each phase. And they are fulfilled when the each phase is completed.
     //
     // In the current module draft, linking will be performed after the whole modules are instantiated and the dependencies are resolved.
@@ -100,7 +92,6 @@ function newRegistryEntry(key)
         state: @ModuleFetch,
         metadata: @undefined,
         fetch: @undefined,
-        translate: @undefined,
         instantiate: @undefined,
         satisfy: @undefined,
         dependencies: [], // To keep the module order, we store the module keys in the array.
@@ -143,18 +134,6 @@ function fulfillFetch(entry, payload)
     if (!entry.fetch)
         entry.fetch = @newPromiseCapability(@InternalPromise).@promise;
     this.forceFulfillPromise(entry.fetch, payload);
-    this.setStateToMax(entry, @ModuleTranslate);
-}
-
-function fulfillTranslate(entry, source)
-{
-    // https://whatwg.github.io/loader/#fulfill-translate
-
-    "use strict";
-
-    if (!entry.translate)
-        entry.translate = @newPromiseCapability(@InternalPromise).@promise;
-    this.forceFulfillPromise(entry.translate, source);
     this.setStateToMax(entry, @ModuleInstantiate);
 }
 
@@ -240,38 +219,13 @@ function requestFetch(key, initiator)
     //     For example, JavaScriptCore shell can provide the hook fetching the resource
     //     from the local file system.
     var fetchPromise = this.fetch(key, initiator).then((payload) => {
-        this.setStateToMax(entry, @ModuleTranslate);
+        this.setStateToMax(entry, @ModuleInstantiate);
         return payload;
     });
     entry.fetch = fetchPromise;
     return fetchPromise;
 }
 
-function requestTranslate(key, initiator)
-{
-    // https://whatwg.github.io/loader/#request-translate
-
-    "use strict";
-
-    var entry = this.ensureRegistered(key);
-    if (entry.translate)
-        return entry.translate;
-
-    var translatePromise = this.requestFetch(key, initiator).then((payload) => {
-        // Hook point.
-        // 3. Loader.translate
-        //     https://whatwg.github.io/loader/#browser-translate
-        //     Take the key and the fetched source code and translate it to the ES6 source code.
-        //     Typically it is used by the transpilers.
-        return this.translate(key, payload, initiator).then((source) => {
-            this.setStateToMax(entry, @ModuleInstantiate);
-            return source;
-        });
-    });
-    entry.translate = translatePromise;
-    return translatePromise;
-}
-
 function requestInstantiate(key, initiator)
 {
     // https://whatwg.github.io/loader/#request-instantiate
@@ -282,11 +236,11 @@ function requestInstantiate(key, initiator)
     if (entry.instantiate)
         return entry.instantiate;
 
-    var instantiatePromise = this.requestTranslate(key, initiator).then((source) => {
+    var instantiatePromise = this.requestFetch(key, initiator).then((source) => {
         // Hook point.
-        // 4. Loader.instantiate
+        // 3. Loader.instantiate
         //     https://whatwg.github.io/loader/#browser-instantiate
-        //     Take the key and the translated source code, and instantiate the module record
+        //     Take the key and the fetched source code, and instantiate the module record
         //     by parsing the module source code.
         //     It has the chance to provide the optional module instance that is different from
         //     the ordinary one.
@@ -462,20 +416,11 @@ function provide(key, stage, value)
         return;
     }
 
-    if (stage === @ModuleTranslate) {
-        if (entry.state > @ModuleTranslate)
-            @throwTypeError("Requested module is already translated.");
-        this.fulfillFetch(entry, @undefined);
-        this.fulfillTranslate(entry, value);
-        return;
-    }
-
     if (stage === @ModuleInstantiate) {
         if (entry.state > @ModuleInstantiate)
             @throwTypeError("Requested module is already instantiated.");
         this.fulfillFetch(entry, @undefined);
-        this.fulfillTranslate(entry, value);
-        entry.translate.then((source) => {
+        entry.fetch.then((source) => {
             this.fulfillInstantiate(entry, value, source);
         });
         return;
diff --git a/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp b/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp
@@ -56,7 +56,6 @@ BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry(VM& vm)
     m_MAX_STRING_LENGTH.set(m_vm, jsNumber(JSString::MaxLength));
     m_MAX_SAFE_INTEGER.set(m_vm, jsDoubleNumber(maxSafeInteger()));
     m_ModuleFetch.set(m_vm, jsNumber(static_cast<unsigned>(JSModuleLoader::Status::Fetch)));
-    m_ModuleTranslate.set(m_vm, jsNumber(static_cast<unsigned>(JSModuleLoader::Status::Translate)));
     m_ModuleInstantiate.set(m_vm, jsNumber(static_cast<unsigned>(JSModuleLoader::Status::Instantiate)));
     m_ModuleSatisfy.set(m_vm, jsNumber(static_cast<unsigned>(JSModuleLoader::Status::Satisfy)));
     m_ModuleLink.set(m_vm, jsNumber(static_cast<unsigned>(JSModuleLoader::Status::Link)));
diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
@@ -1279,8 +1279,18 @@ class GlobalObject : public JSGlobalObject {
 };
 
 const ClassInfo GlobalObject::s_info = { "global", &JSGlobalObject::s_info, nullptr, CREATE_METHOD_TABLE(GlobalObject) };
-const GlobalObjectMethodTable GlobalObject::s_globalObjectMethodTable = { &supportsRichSourceInfo, &shouldInterruptScript, &javaScriptRuntimeFlags, 0, &shouldInterruptScriptBeforeTimeout, &moduleLoaderResolve, &moduleLoaderFetch, nullptr, nullptr, nullptr, nullptr };
-
+const GlobalObjectMethodTable GlobalObject::s_globalObjectMethodTable = {
+    &supportsRichSourceInfo,
+    &shouldInterruptScript,
+    &javaScriptRuntimeFlags,
+    nullptr,
+    &shouldInterruptScriptBeforeTimeout,
+    &moduleLoaderResolve,
+    &moduleLoaderFetch,
+    nullptr,
+    nullptr,
+    nullptr
+};
 
 GlobalObject::GlobalObject(VM& vm, Structure* structure)
     : JSGlobalObject(vm, structure, &s_globalObjectMethodTable)
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -246,7 +246,18 @@ namespace JSC {
 
 const ClassInfo JSGlobalObject::s_info = { "GlobalObject", &Base::s_info, &globalObjectTable, CREATE_METHOD_TABLE(JSGlobalObject) };
 
-const GlobalObjectMethodTable JSGlobalObject::s_globalObjectMethodTable = { &supportsRichSourceInfo, &shouldInterruptScript, &javaScriptRuntimeFlags, nullptr, &shouldInterruptScriptBeforeTimeout, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr };
+const GlobalObjectMethodTable JSGlobalObject::s_globalObjectMethodTable = {
+    &supportsRichSourceInfo,
+    &shouldInterruptScript,
+    &javaScriptRuntimeFlags,
+    nullptr,
+    &shouldInterruptScriptBeforeTimeout,
+    nullptr,
+    nullptr,
+    nullptr,
+    nullptr,
+    nullptr
+};
 
 /* Source for JSGlobalObject.lut.h
 @begin globalObjectTable
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
@@ -189,9 +189,6 @@ struct GlobalObjectMethodTable {
     typedef JSInternalPromise* (*ModuleLoaderFetchPtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue);
     ModuleLoaderFetchPtr moduleLoaderFetch;
 
-    typedef JSInternalPromise* (*ModuleLoaderTranslatePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
-    ModuleLoaderTranslatePtr moduleLoaderTranslate;
-
     typedef JSInternalPromise* (*ModuleLoaderInstantiatePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
     ModuleLoaderInstantiatePtr moduleLoaderInstantiate;
 
diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp
@@ -169,19 +169,6 @@ JSInternalPromise* JSModuleLoader::fetch(ExecState* exec, JSValue key, JSValue i
     return deferred->promise();
 }
 
-JSInternalPromise* JSModuleLoader::translate(ExecState* exec, JSValue key, JSValue payload, JSValue initiator)
-{
-    if (Options::dumpModuleLoadingState())
-        dataLog("Loader [translate] ", printableModuleKey(exec, key), "\n");
-
-    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
-    if (globalObject->globalObjectMethodTable()->moduleLoaderTranslate)
-        return globalObject->globalObjectMethodTable()->moduleLoaderTranslate(globalObject, exec, this, key, payload, initiator);
-    JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
-    deferred->resolve(exec, payload);
-    return deferred->promise();
-}
-
 JSInternalPromise* JSModuleLoader::instantiate(ExecState* exec, JSValue key, JSValue source, JSValue initiator)
 {
     if (Options::dumpModuleLoadingState())
diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.h b/Source/JavaScriptCore/runtime/JSModuleLoader.h
@@ -40,11 +40,10 @@ class JSModuleLoader : public JSNonFinalObject {
 
     enum Status {
         Fetch = 1,
-        Translate = 2,
-        Instantiate = 3,
-        Satisfy = 4,
-        Link = 5,
-        Ready = 6,
+        Instantiate,
+        Satisfy,
+        Link,
+        Ready,
     };
 
     static JSModuleLoader* create(ExecState* exec, VM& vm, JSGlobalObject* globalObject, Structure* structure)
@@ -70,7 +69,6 @@ class JSModuleLoader : public JSNonFinalObject {
     // Platform dependent hooked APIs.
     JSInternalPromise* resolve(ExecState*, JSValue name, JSValue referrer, JSValue initiator);
     JSInternalPromise* fetch(ExecState*, JSValue key, JSValue initiator);
-    JSInternalPromise* translate(ExecState*, JSValue key, JSValue payload, JSValue initiator);
     JSInternalPromise* instantiate(ExecState*, JSValue key, JSValue source, JSValue initiator);
 
     // Additional platform dependent hooked APIs.
diff --git a/Source/JavaScriptCore/runtime/ModuleLoaderPrototype.cpp b/Source/JavaScriptCore/runtime/ModuleLoaderPrototype.cpp
@@ -51,7 +51,6 @@ static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeEvaluate(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeModuleDeclarationInstantiation(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeResolve(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState*);
-static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeTranslate(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeInstantiate(ExecState*);
 
 }
@@ -71,12 +70,10 @@ const ClassInfo ModuleLoaderPrototype::s_info = { "ModuleLoader", &Base::s_info,
     ensureRegistered               JSBuiltin                                           DontEnum|Function 1
     forceFulfillPromise            JSBuiltin                                           DontEnum|Function 2
     fulfillFetch                   JSBuiltin                                           DontEnum|Function 2
-    fulfillTranslate               JSBuiltin                                           DontEnum|Function 2
     fulfillInstantiate             JSBuiltin                                           DontEnum|Function 2
     commitInstantiated             JSBuiltin                                           DontEnum|Function 3
     instantiation                  JSBuiltin                                           DontEnum|Function 3
     requestFetch                   JSBuiltin                                           DontEnum|Function 2
-    requestTranslate               JSBuiltin                                           DontEnum|Function 2
     requestInstantiate             JSBuiltin                                           DontEnum|Function 2
     requestSatisfy                 JSBuiltin                                           DontEnum|Function 2
     requestInstantiateAll          JSBuiltin                                           DontEnum|Function 2
@@ -94,7 +91,6 @@ const ClassInfo ModuleLoaderPrototype::s_info = { "ModuleLoader", &Base::s_info,
     requestedModules               moduleLoaderPrototypeRequestedModules               DontEnum|Function 1
     resolve                        moduleLoaderPrototypeResolve                        DontEnum|Function 2
     fetch                          moduleLoaderPrototypeFetch                          DontEnum|Function 2
-    translate                      moduleLoaderPrototypeTranslate                      DontEnum|Function 3
     instantiate                    moduleLoaderPrototypeInstantiate                    DontEnum|Function 3
 @end
 */
@@ -203,23 +199,11 @@ EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState* exec)
     return JSValue::encode(loader->fetch(exec, exec->argument(0), exec->argument(1)));
 }
 
-EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeTranslate(ExecState* exec)
-{
-    // Hook point, Loader.translate
-    // https://whatwg.github.io/loader/#browser-translate
-    // Take the key and the fetched source code and translate it to the ES6 source code.
-    // Typically it is used by the transpilers.
-    JSModuleLoader* loader = jsDynamicCast<JSModuleLoader*>(exec->thisValue());
-    if (!loader)
-        return JSValue::encode(jsUndefined());
-    return JSValue::encode(loader->translate(exec, exec->argument(0), exec->argument(1), exec->argument(2)));
-}
-
 EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeInstantiate(ExecState* exec)
 {
     // Hook point, Loader.instantiate
     // https://whatwg.github.io/loader/#browser-instantiate
-    // Take the key and the translated source code, and instantiate the module record
+    // Take the key and the fetched source code, and instantiate the module record
     // by parsing the module source code.
     // It has the chance to provide the optional module instance that is different from
     // the ordinary one.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,13 @@
+2016-12-07  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop translate phase in module loader
+        https://bugs.webkit.org/show_bug.cgi?id=164861
+
+        Reviewed by Saam Barati.
+
+        * bindings/js/JSDOMWindowBase.cpp:
+        * bindings/js/JSWorkerGlobalScopeBase.cpp:
+
 2016-12-07  Keith Rollin  <krollin@apple.com>
 
         Network event record/replay
diff --git a/Source/WebCore/bindings/js/JSDOMWindowBase.cpp b/Source/WebCore/bindings/js/JSDOMWindowBase.cpp
@@ -57,7 +57,18 @@ namespace WebCore {
 
 const ClassInfo JSDOMWindowBase::s_info = { "Window", &JSDOMGlobalObject::s_info, 0, CREATE_METHOD_TABLE(JSDOMWindowBase) };
 
-const GlobalObjectMethodTable JSDOMWindowBase::s_globalObjectMethodTable = { &supportsRichSourceInfo, &shouldInterruptScript, &javaScriptRuntimeFlags, &queueTaskToEventLoop, &shouldInterruptScriptBeforeTimeout, &moduleLoaderResolve, &moduleLoaderFetch, nullptr, nullptr, &moduleLoaderEvaluate, &defaultLanguage };
+const GlobalObjectMethodTable JSDOMWindowBase::s_globalObjectMethodTable = {
+    &supportsRichSourceInfo,
+    &shouldInterruptScript,
+    &javaScriptRuntimeFlags,
+    &queueTaskToEventLoop,
+    &shouldInterruptScriptBeforeTimeout,
+    &moduleLoaderResolve,
+    &moduleLoaderFetch,
+    nullptr,
+    &moduleLoaderEvaluate,
+    &defaultLanguage
+};
 
 JSDOMWindowBase::JSDOMWindowBase(VM& vm, Structure* structure, RefPtr<DOMWindow>&& window, JSDOMWindowShell* shell)
     : JSDOMGlobalObject(vm, structure, shell->world(), &s_globalObjectMethodTable)
diff --git a/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp b/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp
