Web Inspector: allow import() inside the inspector
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2017 17:32:12 +0000 (17:32 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2017 17:32:12 +0000 (17:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167457

Reviewed by Ryosuke Niwa.

Source/JavaScriptCore:

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):

Source/WebCore:

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.

Test: inspector/controller/runtime-controller-import.html

* bindings/js/ScriptModuleLoader.cpp:
(WebCore::ScriptModuleLoader::importModule):

LayoutTests:

Extract the test to single file. And make it deterministic.

* inspector/controller/resources/cappuccino.js: Added.
* inspector/controller/resources/cocoa.js: Added.
* inspector/controller/resources/drink.js: Added.
* inspector/controller/runtime-controller-import-expected.txt: Added.
* inspector/controller/runtime-controller-import.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@212438 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/controller/resources/cappuccino.js [new file with mode: 0644]
LayoutTests/inspector/controller/resources/cocoa.js [new file with mode: 0644]
LayoutTests/inspector/controller/resources/drink.js [new file with mode: 0644]
LayoutTests/inspector/controller/runtime-controller-import-expected.txt [new file with mode: 0644]
LayoutTests/inspector/controller/runtime-controller-import.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/ScriptModuleLoader.cpp

index 1ca5d26..3b9a20c 100644 (file)
@@ -1,3 +1,18 @@
+2017-02-16  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Web Inspector: allow import() inside the inspector
+        https://bugs.webkit.org/show_bug.cgi?id=167457
+
+        Reviewed by Ryosuke Niwa.
+
+        Extract the test to single file. And make it deterministic.
+
+        * inspector/controller/resources/cappuccino.js: Added.
+        * inspector/controller/resources/cocoa.js: Added.
+        * inspector/controller/resources/drink.js: Added.
+        * inspector/controller/runtime-controller-import-expected.txt: Added.
+        * inspector/controller/runtime-controller-import.html: Added.
+
 2017-02-16  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed GTK+ gardening. Rebaseline http/tests/xmlhttprequest/xmlhttprequest-test-custom-headers.html.
diff --git a/LayoutTests/inspector/controller/resources/cappuccino.js b/LayoutTests/inspector/controller/resources/cappuccino.js
new file mode 100644 (file)
index 0000000..43b9430
--- /dev/null
@@ -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 (file)
index 0000000..ee57233
--- /dev/null
@@ -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 (file)
index 0000000..b2d9a89
--- /dev/null
@@ -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-import-expected.txt b/LayoutTests/inspector/controller/runtime-controller-import-expected.txt
new file mode 100644 (file)
index 0000000..fd51a29
--- /dev/null
@@ -0,0 +1,20 @@
+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.
+
+
+== Running test suite: RuntimeManager
+-- 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-import.html b/LayoutTests/inspector/controller/runtime-controller-import.html
new file mode 100644 (file)
index 0000000..0ec651c
--- /dev/null
@@ -0,0 +1,49 @@
+<!doctype html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("RuntimeManager");
+
+    suite.addTestCase({
+        name: "RuntimeManager.prototype.evaluateInInspectedWindow.ImportFromConsole",
+        description: "Test evaluating an import expression from console.",
+        test(resolve, reject) {
+            function testSource(expression, count) {
+                let promise = WebInspector.logManager.awaitEvent(WebInspector.LogManager.Event.MessageAdded);
+                return new Promise((resolve, reject) => {
+                    WebInspector.runtimeManager.evaluateInInspectedWindow(expression, {objectGroup: "test"}, (result, wasThrown) => {
+                        promise.then(() => {
+                            InspectorTest.log("Source: " + expression);
+                            InspectorTest.expectThat(result.isUndefined(), "Transformed. Should log the value or an exception.");
+                            resolve();
+                        });
+                    });
+                })
+            }
+
+            testSource("await import('')", 0)
+            .then(() => {
+                return testSource("await import('./not-found.js')");
+            })
+            .then(() => {
+                testSource("await import('./resources/cocoa.js').then((cocoa) => { console.log(`${cocoa.name} is ${cocoa.taste}.`); })");
+            })
+            .then(() => {
+                testSource("await import('./resources/drink.js').then((ns) => { console.log(`${ns.Cocoa.name} is ${ns.Cocoa.taste}. ${ns.Cappuccino.name} is ${ns.Cappuccino.taste}.`); })");
+            })
+            .then(resolve);
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Tests for RuntimeManager operations.</p>
+</body>
+</html>
index aa68b52..85d5019 100644 (file)
@@ -1,5 +1,20 @@
 2017-02-16  Yusuke Suzuki  <utatane.tea@gmail.com>
 
+        Web Inspector: allow import() inside the inspector
+        https://bugs.webkit.org/show_bug.cgi?id=167457
+
+        Reviewed by Ryosuke Niwa.
+
+        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-16  Yusuke Suzuki  <utatane.tea@gmail.com>
+
         [JSC] Update module namespace object according to the latest ECMA262
         https://bugs.webkit.org/show_bug.cgi?id=168280
 
index bda9a09..16c0428 100644 (file)
@@ -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())) {
index f3f76e6..e3a4146 100644 (file)
@@ -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()) {
index 32651cd..94395e1 100644 (file)
@@ -1,3 +1,35 @@
+2017-02-16  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Web Inspector: allow import() inside the inspector
+        https://bugs.webkit.org/show_bug.cgi?id=167457
+
+        Reviewed by Ryosuke Niwa.
+
+        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.
+
+        Test: inspector/controller/runtime-controller-import.html
+
+        * bindings/js/ScriptModuleLoader.cpp:
+        (WebCore::ScriptModuleLoader::importModule):
+
 2017-02-16  Miguel Gomez  <magomez@igalia.com>
 
         [GTK] scroll with transparent background not repainted after scrollY >= 32768
index 1ce7253..b67ffa3 100644 (file)
@@ -218,22 +218,37 @@ 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.
+    //     3. The code from injected bundle's script.
+    //     4. The code from extension script.
+    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)