Attachment 300397 Details for Bug 167457 – Patch

[patch] Patch

bug-167457-20170202202759.patch (text/plain), 12.66 KB, created by Yusuke Suzuki on 2017-02-02 03:30:13 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2017-02-02 03:30:13 PST

Size: 12.66 KB

patch

obsolete

>Subversion Revision: 211554
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index b9e0d9f39066db2db9db2a082983ce087ba52b33..d484838974c2d264b48b162e3152cd5c0add926a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,18 @@
>+2017-02-02  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Web Inspector: allow import() inside the inspector
>+        https://bugs.webkit.org/show_bug.cgi?id=167457
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We relax import module hook to accept null SourceOrigin.
>+        Such a script can be evaluated from the inspector console.
>+
>+        * jsc.cpp:
>+        (GlobalObject::moduleLoaderImportModule):
>+        * runtime/JSGlobalObjectFunctions.cpp:
>+        (JSC::globalFuncImportModule):
>+
> 2017-02-02  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Removed unused m_errorHandlingModeReentry from Interpreter
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 70b353b80a16a919ad02848cd0942fb61d189222..7e3b90c114e5974c9fe22d3ebdc4a7e468acaded 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2017-02-02  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Web Inspector: allow import() inside the inspector
>+        https://bugs.webkit.org/show_bug.cgi?id=167457
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When evaluating `import("...")`, we need the caller's context to resolve
>+        the module specifier correctly. For example, if `import("./cocoa.js")` is
>+        evaluated in the script "drinks/hot.js", this module name is resolved to
>+        "drinks/cocoa.js". If the same import operator is evaluated in the script
>+        "menu/all.js", the module specifier becomes "menu/cocoa.js".
>+
>+        Previously we reject the import operator if the caller does not have such
>+        a context. These context is SourceOrigin and its ScriptFetcher. While they
>+        are offered in the script tag and other code evaluations, the inspector
>+        console does not offer that. These class are offered in the WebCore side
>+        and we should not touch these classes in the JSC's inspector code.
>+
>+        Now we relax the above restriction. If the above caller information is not
>+        offered, we fallback to the default one. In the web page, we use the page's
>+        URL as the caller's source origin. This allows us to evaluate the import
>+        operator in the inspector console.
>+
>+        And as of r167698, the console recognizes `await import("...")` form. We use
>+        this to test this `import()` in the console functionality.
>+
>+        * bindings/js/ScriptModuleLoader.cpp:
>+        (WebCore::ScriptModuleLoader::importModule):
>+
> 2017-02-02  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Removed unused m_navigationStart from NavigatorGamepad
>diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
>index bda9a09d0d26b96c3f50a2299d24933ac74f3056..16c0428a9adccc8de444116c2028ba7a84d8c453 100644
>--- a/Source/JavaScriptCore/jsc.cpp
>+++ b/Source/JavaScriptCore/jsc.cpp
>@@ -1551,6 +1551,9 @@ JSInternalPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* global
>         return JSInternalPromiseDeferred::create(exec, globalObject)->reject(exec, error);
>     };
> 
>+    if (sourceOrigin.isNull())
>+        return rejectPromise(createError(exec, ASCIILiteral("Could not resolve the module specifier.")));
>+
>     auto referrer = sourceOrigin.string();
>     auto moduleName = moduleNameValue->value(exec);
>     if (UNLIKELY(scope.exception())) {
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp b/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
>index f3f76e6a932646a42c9a81fc91b202084e6864d7..e3a4146824982761136ec7bb8b709530172638cf 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
>@@ -944,11 +944,6 @@ EncodedJSValue JSC_HOST_CALL globalFuncImportModule(ExecState* exec)
>     RETURN_IF_EXCEPTION(catchScope, { });
> 
>     auto sourceOrigin = exec->callerSourceOrigin();
>-    if (sourceOrigin.isNull()) {
>-        promise->reject(exec, createError(exec, ASCIILiteral("Could not resolve the module specifier.")));
>-        return JSValue::encode(promise->promise());
>-    }
>-
>     RELEASE_ASSERT(exec->argumentCount() == 1);
>     auto* specifier = exec->uncheckedArgument(0).toString(exec);
>     if (Exception* exception = catchScope.exception()) {
>diff --git a/Source/WebCore/bindings/js/ScriptModuleLoader.cpp b/Source/WebCore/bindings/js/ScriptModuleLoader.cpp
>index 1ce7253e2fec8fc0195d24f0d66d988657d35818..438e35890307c65d05e521eeec4205c8b5e33347 100644
>--- a/Source/WebCore/bindings/js/ScriptModuleLoader.cpp
>+++ b/Source/WebCore/bindings/js/ScriptModuleLoader.cpp
>@@ -218,22 +218,35 @@ JSC::JSInternalPromise* ScriptModuleLoader::importModule(JSC::JSGlobalObject* js
>     JSC::VM& vm = exec->vm();
>     auto& globalObject = *JSC::jsCast<JSDOMGlobalObject*>(jsGlobalObject);
> 
>-    // FIXME: setTimeout and setInterval with "string()" should propagate SourceOrigin.
>-    // https://webkit.org/b/167097
>-    ASSERT_WITH_MESSAGE(!sourceOrigin.isNull(), "If SourceOrigin is null, this function is not invoked.");
>-    if (!sourceOrigin.fetcher())
>-        return rejectPromise(state, globalObject, TypeError, ASCIILiteral("Could not use import operator in this context."));
>-
>-    URL baseURL(URL(), sourceOrigin.string());
>-    if (!baseURL.isValid())
>-        return rejectPromise(state, globalObject, TypeError, ASCIILiteral("Importer module key is not Symbol or String."));
>+    // If SourceOrigin and/or CachedScriptFetcher is null, we import the module with the default fetcher.
>+    // SourceOrigin can be null if the source code is not coupled with the script file.
>+    // The examples,
>+    //     1. The code evaluated by the inspector.
>+    //     2. The other unusual code execution like the evaluation through the NPAPI.
>+    URL baseURL;
>+    RefPtr<JSC::ScriptFetcher> scriptFetcher;
>+    if (sourceOrigin.isNull()) {
>+        baseURL = m_document.baseURL();
>+        scriptFetcher = CachedScriptFetcher::create(m_document.charset());
>+    } else {
>+        baseURL = URL(URL(), sourceOrigin.string());
>+        if (!baseURL.isValid())
>+            return rejectPromise(state, globalObject, TypeError, ASCIILiteral("Importer module key is not a Symbol or a String."));
>+
>+        if (sourceOrigin.fetcher())
>+            scriptFetcher = sourceOrigin.fetcher();
>+        else
>+            scriptFetcher = CachedScriptFetcher::create(m_document.charset());
>+    }
>+    ASSERT(baseURL.isValid());
>+    ASSERT(scriptFetcher);
> 
>     auto specifier = moduleName->value(exec);
>     auto result = resolveModuleSpecifier(m_document, specifier, baseURL);
>     if (!result)
>         return rejectPromise(state, globalObject, TypeError, result.error());
> 
>-    return JSC::importModule(exec, JSC::Identifier::fromString(&vm, result->string()), JSC::JSScriptFetcher::create(vm, sourceOrigin.fetcher() ));
>+    return JSC::importModule(exec, JSC::Identifier::fromString(&vm, result->string()), JSC::JSScriptFetcher::create(vm, WTFMove(scriptFetcher) ));
> }
> 
> void ScriptModuleLoader::notifyFinished(CachedModuleScriptLoader& loader, RefPtr<DeferredPromise> promise)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6a91bb564fb8cf48214fa56b17e258125ecbdb39..9250fe224036519709bdc4c123af44655d9e6ab7 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2017-02-02  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Web Inspector: allow import() inside the inspector
>+        https://bugs.webkit.org/show_bug.cgi?id=167457
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/controller/resources/cappuccino.js: Added.
>+        * inspector/controller/resources/cocoa.js: Added.
>+        * inspector/controller/resources/drink.js: Added.
>+        * inspector/controller/runtime-controller-expected.txt:
>+        * inspector/controller/runtime-controller.html:
>+
> 2017-02-01  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         Web Inspector: Upgrade Esprima to the latest one to support dynamic import
>diff --git a/LayoutTests/inspector/controller/resources/cappuccino.js b/LayoutTests/inspector/controller/resources/cappuccino.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..43b94301ef84050d017cc268d1262d0e1cc4557d
>--- /dev/null
>+++ b/LayoutTests/inspector/controller/resources/cappuccino.js
>@@ -0,0 +1,2 @@
>+export let name = "Cappuccino";
>+export let taste = "Awesome";
>diff --git a/LayoutTests/inspector/controller/resources/cocoa.js b/LayoutTests/inspector/controller/resources/cocoa.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..ee5723326a5fe4122685b9090f8d57d8419d6ef9
>--- /dev/null
>+++ b/LayoutTests/inspector/controller/resources/cocoa.js
>@@ -0,0 +1,2 @@
>+export let name = "Cocoa";
>+export let taste = "Sweet";
>diff --git a/LayoutTests/inspector/controller/resources/drink.js b/LayoutTests/inspector/controller/resources/drink.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..b2d9a89d8cd5e19d202ea8858351c62272c4bbe5
>--- /dev/null
>+++ b/LayoutTests/inspector/controller/resources/drink.js
>@@ -0,0 +1,7 @@
>+import * as Cocoa from './cocoa.js'
>+import * as Cappuccino from './cappuccino.js'
>+
>+export {
>+    Cocoa,
>+    Cappuccino
>+}
>diff --git a/LayoutTests/inspector/controller/runtime-controller-expected.txt b/LayoutTests/inspector/controller/runtime-controller-expected.txt
>index ded82febae3988999aa37588fbb986152278f73d..27ef571165a191645174027b546519e3bf517910 100644
>--- a/LayoutTests/inspector/controller/runtime-controller-expected.txt
>+++ b/LayoutTests/inspector/controller/runtime-controller-expected.txt
>@@ -7,6 +7,12 @@ CONSOLE MESSAGE: line 9: Promise.reject
> CONSOLE MESSAGE: line 9: Rejection
> CONSOLE MESSAGE: line 7: %o
> CONSOLE MESSAGE: line 7: %o
>+CONSOLE MESSAGE: line 9: TypeError: Module specifier does not start with "/", "./", or "../".
>+CONSOLE MESSAGE: line 9: TypeError: Importing a module script failed.
>+CONSOLE MESSAGE: line 6: Cocoa is Sweet.
>+CONSOLE MESSAGE: line 7: %o
>+CONSOLE MESSAGE: line 6: Cocoa is Sweet. Cappuccino is Awesome.
>+CONSOLE MESSAGE: line 7: %o
> Tests for RuntimeManager operations.
> 
> 
>@@ -58,3 +64,13 @@ PASS: Exception. Should not get transformed and produce a SyntaxError.
> Source: x = y = await 10
> PASS: Exception. Should not get transformed and produce a SyntaxError.
> 
>+-- Running test case: RuntimeManager.prototype.evaluateInInspectedWindow.ImportFromConsole
>+Source: await import('')
>+PASS: Transformed. Should log the value or an exception.
>+Source: await import('./not-found.js')
>+PASS: Transformed. Should log the value or an exception.
>+Source: await import('./resources/cocoa.js').then((cocoa) => console.log(`${cocoa.name} is ${cocoa.taste}.`))
>+PASS: Transformed. Should log the value or an exception.
>+Source: await import('./resources/drink.js').then((ns) => console.log(`${ns.Cocoa.name} is ${ns.Cocoa.taste}. ${ns.Cappuccino.name} is ${ns.Cappuccino.taste}.`))
>+PASS: Transformed. Should log the value or an exception.
>+
>diff --git a/LayoutTests/inspector/controller/runtime-controller.html b/LayoutTests/inspector/controller/runtime-controller.html
>index 4ca23f3254a8eb2746b840144ddd301817165d05..4dbac62f05660d37e1d7a996ffc209c8b93a2bea 100644
>--- a/LayoutTests/inspector/controller/runtime-controller.html
>+++ b/LayoutTests/inspector/controller/runtime-controller.html
>@@ -124,6 +124,30 @@
>         }
>     });
> 
>+    suite.addTestCase({
>+        name: "RuntimeManager.prototype.evaluateInInspectedWindow.ImportFromConsole",
>+        description: "Test evaluating an import expression from console.",
>+        test(resolve, reject) {
>+            function testSource(expression, callback) {
>+                WebInspector.runtimeManager.evaluateInInspectedWindow(expression, {objectGroup: "test"}, (result, wasThrown) => {
>+                    InspectorTest.log("Source: " + expression);
>+                    callback(result, wasThrown);
>+                });
>+            }
>+
>+            function expectUndefined(result, wasThrown) {
>+                InspectorTest.expectThat(result.isUndefined(), "Transformed. Should log the value or an exception.");
>+            }
>+
>+            testSource("await import('')", expectUndefined);
>+            testSource("await import('./not-found.js')", expectUndefined);
>+            testSource("await import('./resources/cocoa.js').then((cocoa) => console.log(`${cocoa.name} is ${cocoa.taste}.`))", expectUndefined);
>+            testSource("await import('./resources/drink.js').then((ns) => console.log(`${ns.Cocoa.name} is ${ns.Cocoa.taste}. ${ns.Cappuccino.name} is ${ns.Cappuccino.taste}.`))", expectUndefined);
>+
>+            InspectorBackend.runAfterPendingDispatches(resolve);
>+        }
>+    });
>+
>     suite.runTestCasesAndFinish();
> }
> </script>

Actions: View | Formatted Diff | Diff

Attachments on bug 167457: 300335 | 300397 | 300531 | 300532