Changeset 223777 in webkit


Ignore:
Timestamp:
Oct 20, 2017 10:58:32 AM (6 years ago)
Author:
Matt Lewis
Message:

Unreviewed, rolling out r223744, r223750, and r223751.
https://bugs.webkit.org/show_bug.cgi?id=178594

These caused consistent failures in test that existed and were
added in the patches. (Requested by mlewis13 on #webkit).

Reverted changesets:

"[JSC] ScriptFetcher should be notified directly from module
pipeline"
https://bugs.webkit.org/show_bug.cgi?id=178340
https://trac.webkit.org/changeset/223744

"Unreviewed, fix changed line number in test expect files"
https://bugs.webkit.org/show_bug.cgi?id=178340
https://trac.webkit.org/changeset/223750

"Unreviewed, follow up to reflect comments"
https://bugs.webkit.org/show_bug.cgi?id=178340
https://trac.webkit.org/changeset/223751

Patch by Commit Queue <commit-queue@webkit.org> on 2017-10-20

Location:
trunk
Files:
18 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r223769 r223777  
     12017-10-20  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r223744, r223750, and r223751.
     4        https://bugs.webkit.org/show_bug.cgi?id=178594
     5
     6        These caused consistent failures in test that existed and were
     7        added in the patches. (Requested by mlewis13 on #webkit).
     8
     9        Reverted changesets:
     10
     11        "[JSC] ScriptFetcher should be notified directly from module
     12        pipeline"
     13        https://bugs.webkit.org/show_bug.cgi?id=178340
     14        https://trac.webkit.org/changeset/223744
     15
     16        "Unreviewed, fix changed line number in test expect files"
     17        https://bugs.webkit.org/show_bug.cgi?id=178340
     18        https://trac.webkit.org/changeset/223750
     19
     20        "Unreviewed, follow up to reflect comments"
     21        https://bugs.webkit.org/show_bug.cgi?id=178340
     22        https://trac.webkit.org/changeset/223751
     23
    1242017-10-20  Zan Dobersek  <zdobersek@igalia.com>
    225
  • trunk/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/module-scriptnonce-redirect-expected.txt

    r223750 r223777  
    11CONSOLE MESSAGE: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
    2 CONSOLE MESSAGE: line 16: TypeError: Cross-origin script load denied by Cross-Origin Resource Sharing policy.
     2CONSOLE MESSAGE: line 1: TypeError: Cross-origin script load denied by Cross-Origin Resource Sharing policy.
    33This tests whether a deferred script load caused by a redirect is properly allowed by a nonce.
  • trunk/LayoutTests/http/tests/security/module-no-mime-type-expected.txt

    r223750 r223777  
    1 CONSOLE MESSAGE: line 16: TypeError: 'application/octet-stream' is not a valid JavaScript MIME type.
     1CONSOLE MESSAGE: line 1: TypeError: 'application/octet-stream' is not a valid JavaScript MIME type.
    22Test module rejects scripts with no mime type.
    33
  • trunk/LayoutTests/js/dom/modules/module-execution-error-should-be-propagated-to-onerror-expected.txt

    r223744 r223777  
    1 CONSOLE MESSAGE: line 19: Error: module is executed.
    21Test window.onerror will be fired when the inlined module throws an error.
    32
  • trunk/Source/JavaScriptCore/ChangeLog

    r223751 r223777  
     12017-10-20  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r223744, r223750, and r223751.
     4        https://bugs.webkit.org/show_bug.cgi?id=178594
     5
     6        These caused consistent failures in test that existed and were
     7        added in the patches. (Requested by mlewis13 on #webkit).
     8
     9        Reverted changesets:
     10
     11        "[JSC] ScriptFetcher should be notified directly from module
     12        pipeline"
     13        https://bugs.webkit.org/show_bug.cgi?id=178340
     14        https://trac.webkit.org/changeset/223744
     15
     16        "Unreviewed, fix changed line number in test expect files"
     17        https://bugs.webkit.org/show_bug.cgi?id=178340
     18        https://trac.webkit.org/changeset/223750
     19
     20        "Unreviewed, follow up to reflect comments"
     21        https://bugs.webkit.org/show_bug.cgi?id=178340
     22        https://trac.webkit.org/changeset/223751
     23
    1242017-10-20  Yusuke Suzuki  <utatane.tea@gmail.com>
    225
  • trunk/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js

    r223744 r223777  
    321321    return this.resolve(moduleName, @undefined, fetcher).then((key) => {
    322322        return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher);
    323     }).then(
    324         (entry) => {
    325             this.notifyCompleted(fetcher, entry.key);
    326             return entry;
    327         },
    328         (error) => {
    329             this.notifyFailed(fetcher, error);
    330             throw error;
    331         });
     323    }).then((entry) => {
     324        return entry.key;
     325    });
    332326}
    333327
     
    348342    "use strict";
    349343
    350     return this.loadModule(moduleName, parameters, fetcher).then((entry) => {
    351         return this.linkAndEvaluateModule(entry.key, fetcher);
     344    return this.loadModule(moduleName, parameters, fetcher).then((key) => {
     345        return this.linkAndEvaluateModule(key, fetcher);
    352346    });
    353347}
  • trunk/Source/JavaScriptCore/runtime/Completion.cpp

    r223744 r223777  
    184184}
    185185
    186 void loadModule(ExecState* exec, const String& moduleName, JSValue parameters, JSValue scriptFetcher)
    187 {
    188     VM& vm = exec->vm();
    189     JSLockHolder lock(vm);
    190     RELEASE_ASSERT(vm.atomicStringTable() == Thread::current().atomicStringTable());
    191     RELEASE_ASSERT(!vm.isCollectorBusyOnCurrentThread());
    192 
    193     exec->vmEntryGlobalObject()->moduleLoader()->loadModule(exec, identifierToJSValue(vm, Identifier::fromString(exec, moduleName)), parameters, scriptFetcher);
    194 }
    195 
    196 void loadModule(ExecState* exec, const SourceCode& source, JSValue scriptFetcher)
     186JSInternalPromise* loadModule(ExecState* exec, const String& moduleName, JSValue parameters, JSValue scriptFetcher)
     187{
     188    VM& vm = exec->vm();
     189    JSLockHolder lock(vm);
     190    RELEASE_ASSERT(vm.atomicStringTable() == Thread::current().atomicStringTable());
     191    RELEASE_ASSERT(!vm.isCollectorBusyOnCurrentThread());
     192
     193    return exec->vmEntryGlobalObject()->moduleLoader()->loadModule(exec, identifierToJSValue(vm, Identifier::fromString(exec, moduleName)), parameters, scriptFetcher);
     194}
     195
     196JSInternalPromise* loadModule(ExecState* exec, const SourceCode& source, JSValue scriptFetcher)
    197197{
    198198    VM& vm = exec->vm();
     
    209209    // FIXME: Introduce JSSourceCode object to wrap around this source.
    210210    globalObject->moduleLoader()->provideFetch(exec, key, source);
    211     RETURN_IF_EXCEPTION(scope, void());
    212 
    213     globalObject->moduleLoader()->loadModule(exec, key, jsUndefined(), scriptFetcher);
     211    RETURN_IF_EXCEPTION(scope, rejectPromise(exec, globalObject));
     212
     213    return globalObject->moduleLoader()->loadModule(exec, key, jsUndefined(), scriptFetcher);
    214214}
    215215
  • trunk/Source/JavaScriptCore/runtime/Completion.h

    r223744 r223777  
    6262
    6363// Fetch the module source, and instantiate the module record.
    64 JS_EXPORT_PRIVATE void loadModule(ExecState*, const String& moduleName, JSValue parameters, JSValue scriptFetcher);
    65 JS_EXPORT_PRIVATE void loadModule(ExecState*, const SourceCode&, JSValue scriptFetcher);
     64JS_EXPORT_PRIVATE JSInternalPromise* loadModule(ExecState*, const String& moduleName, JSValue parameters, JSValue scriptFetcher);
     65JS_EXPORT_PRIVATE JSInternalPromise* loadModule(ExecState*, const SourceCode&, JSValue scriptFetcher);
    6666
    6767// Link and evaluate the already linked module. This function is called in a sync manner.
  • trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp

    r223751 r223777  
    4040#include "JSModuleEnvironment.h"
    4141#include "JSModuleRecord.h"
    42 #include "JSScriptFetcher.h"
    4342#include "JSSourceCode.h"
    4443#include "ModuleAnalyzer.h"
     
    293292}
    294293
    295 static Identifier jsValueToModuleKey(ExecState* exec, JSValue value)
    296 {
    297     if (value.isSymbol())
    298         return Identifier::fromUid(jsCast<Symbol*>(value)->privateName());
    299     ASSERT(value.isString());
    300     return asString(value)->toIdentifier(exec);
    301 }
    302 
    303 JSValue JSModuleLoader::notifyCompleted(ExecState* exec, JSValue scriptFetcher, JSValue key)
    304 {
    305     auto* fetcherWrapper = jsDynamicCast<JSScriptFetcher*>(exec->vm(), scriptFetcher);
    306     if (!fetcherWrapper)
    307         return jsUndefined();
    308     auto* fetcher = fetcherWrapper->fetcher();
    309     if (!fetcher)
    310         return jsUndefined();
    311 
    312     auto moduleKey = jsValueToModuleKey(exec, key);
    313     fetcher->notifyLoadCompleted(*moduleKey.impl());
    314     return jsUndefined();
    315 }
    316 
    317 JSValue JSModuleLoader::notifyFailed(ExecState* exec, JSValue scriptFetcher, JSValue errorValue)
    318 {
    319     auto* fetcherWrapper = jsDynamicCast<JSScriptFetcher*>(exec->vm(), scriptFetcher);
    320     if (!fetcherWrapper)
    321         return jsUndefined();
    322     auto* fetcher = fetcherWrapper->fetcher();
    323     if (!fetcher)
    324         return jsUndefined();
    325     fetcher->notifyLoadFailed(exec, errorValue);
    326     return jsUndefined();
    327 }
    328 
    329294} // namespace JSC
  • trunk/Source/JavaScriptCore/runtime/JSModuleLoader.h

    r223744 r223777  
    6969    JSValue linkAndEvaluateModule(ExecState*, JSValue moduleKey, JSValue scriptFetcher);
    7070    JSInternalPromise* requestImportModule(ExecState*, const Identifier&, JSValue parameters, JSValue scriptFetcher);
    71     JSValue notifyCompleted(ExecState*, JSValue scriptFetcher, JSValue key);
    72     JSValue notifyFailed(ExecState*, JSValue scriptFetcher, JSValue error);
    7371
    7472    // Platform dependent hooked APIs.
  • trunk/Source/JavaScriptCore/runtime/ModuleLoaderPrototype.cpp

    r223744 r223777  
    5757static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState*);
    5858static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeGetModuleNamespaceObject(ExecState*);
    59 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeNotifyCompleted(ExecState*);
    60 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeNotifyFailed(ExecState*);
    6159
    6260}
     
    9391    resolveSync                    moduleLoaderPrototypeResolveSync                    DontEnum|Function 2
    9492    fetch                          moduleLoaderPrototypeFetch                          DontEnum|Function 3
    95     notifyCompleted                moduleLoaderPrototypeNotifyCompleted                DontEnum|Function 2
    96     notifyFailed                   moduleLoaderPrototypeNotifyFailed                   DontEnum|Function 2
    9793@end
    9894*/
     
    175171}
    176172
    177 EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeNotifyCompleted(ExecState* exec)
    178 {
    179     VM& vm = exec->vm();
    180     JSModuleLoader* loader = jsDynamicCast<JSModuleLoader*>(vm, exec->thisValue());
    181     if (!loader)
    182         return JSValue::encode(jsUndefined());
    183     return JSValue::encode(loader->notifyCompleted(exec, exec->argument(0), exec->argument(1)));
    184 }
    185 
    186 EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeNotifyFailed(ExecState* exec)
    187 {
    188     VM& vm = exec->vm();
    189     JSModuleLoader* loader = jsDynamicCast<JSModuleLoader*>(vm, exec->thisValue());
    190     if (!loader)
    191         return JSValue::encode(jsUndefined());
    192     return JSValue::encode(loader->notifyFailed(exec, exec->argument(0), exec->argument(1)));
    193 }
    194 
    195173// ------------------------------ Hook Functions ---------------------------
    196174
  • trunk/Source/JavaScriptCore/runtime/ScriptFetcher.h

    r223744 r223777  
    2626#pragma once
    2727
    28 #include "JSCJSValue.h"
    2928#include <wtf/RefCounted.h>
    3029
    3130namespace JSC {
    3231
    33 class ExecState;
    34 
    3532class ScriptFetcher : public RefCounted<ScriptFetcher> {
    3633public:
    3734    virtual ~ScriptFetcher() { }
    38 
    39     virtual void notifyLoadCompleted(UniquedStringImpl&) { }
    40     virtual void notifyLoadFailed(ExecState*, JSValue) { }
    4135};
    4236
  • trunk/Source/WebCore/ChangeLog

    r223776 r223777  
     12017-10-20  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r223744, r223750, and r223751.
     4        https://bugs.webkit.org/show_bug.cgi?id=178594
     5
     6        These caused consistent failures in test that existed and were
     7        added in the patches. (Requested by mlewis13 on #webkit).
     8
     9        Reverted changesets:
     10
     11        "[JSC] ScriptFetcher should be notified directly from module
     12        pipeline"
     13        https://bugs.webkit.org/show_bug.cgi?id=178340
     14        https://trac.webkit.org/changeset/223744
     15
     16        "Unreviewed, fix changed line number in test expect files"
     17        https://bugs.webkit.org/show_bug.cgi?id=178340
     18        https://trac.webkit.org/changeset/223750
     19
     20        "Unreviewed, follow up to reflect comments"
     21        https://bugs.webkit.org/show_bug.cgi?id=178340
     22        https://trac.webkit.org/changeset/223751
     23
    1242017-10-20  Zalan Bujtas  <zalan@apple.com>
    225
  • trunk/Source/WebCore/bindings/js/JSMainThreadExecState.h

    r223744 r223777  
    9191    }
    9292
    93     static void loadModule(JSC::ExecState& state, const String& moduleName, JSC::JSValue parameters, JSC::JSValue scriptFetcher)
     93    static JSC::JSInternalPromise& loadModule(JSC::ExecState& state, const String& moduleName, JSC::JSValue parameters, JSC::JSValue scriptFetcher)
    9494    {
    9595        JSMainThreadExecState currentState(&state);
    96         JSC::loadModule(&state, moduleName, parameters, scriptFetcher);
     96        return *JSC::loadModule(&state, moduleName, parameters, scriptFetcher);
    9797    }
    9898
    99     static void loadModule(JSC::ExecState& state, const JSC::SourceCode& sourceCode, JSC::JSValue scriptFetcher)
     99    static JSC::JSInternalPromise& loadModule(JSC::ExecState& state, const JSC::SourceCode& sourceCode, JSC::JSValue scriptFetcher)
    100100    {
    101101        JSMainThreadExecState currentState(&state);
    102         JSC::loadModule(&state, sourceCode, scriptFetcher);
     102        return *JSC::loadModule(&state, sourceCode, scriptFetcher);
    103103    }
    104104
  • trunk/Source/WebCore/bindings/js/ScriptController.cpp

    r223744 r223777  
    4141#include "LoadableModuleScript.h"
    4242#include "MainFrame.h"
     43#include "ModuleFetchFailureKind.h"
    4344#include "ModuleFetchParameters.h"
    4445#include "NP_jsobject.h"
     
    6263#include <runtime/JSLock.h>
    6364#include <runtime/JSModuleRecord.h>
     65#include <runtime/JSNativeStdFunction.h>
    6466#include <runtime/JSScriptFetchParameters.h>
    6567#include <runtime/JSScriptFetcher.h>
     
    198200    auto& state = *proxy.window()->globalExec();
    199201
    200     JSMainThreadExecState::loadModule(state, moduleName, JSC::JSScriptFetchParameters::create(state.vm(), WTFMove(topLevelFetchParameters)), JSC::JSScriptFetcher::create(state.vm(), { &moduleScript }));
     202    auto& promise = JSMainThreadExecState::loadModule(state, moduleName, JSC::JSScriptFetchParameters::create(state.vm(), WTFMove(topLevelFetchParameters)), JSC::JSScriptFetcher::create(state.vm(), { &moduleScript }));
     203    setupModuleScriptHandlers(moduleScript, promise, world);
    201204}
    202205
     
    213216    auto& state = *proxy.window()->globalExec();
    214217
    215     JSMainThreadExecState::loadModule(state, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(state.vm(), { &moduleScript }));
     218    auto& promise = JSMainThreadExecState::loadModule(state, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(state.vm(), { &moduleScript }));
     219    setupModuleScriptHandlers(moduleScript, promise, world);
    216220}
    217221
     
    361365}
    362366
     367static Identifier jsValueToModuleKey(ExecState* exec, JSValue value)
     368{
     369    if (value.isSymbol())
     370        return Identifier::fromUid(jsCast<Symbol*>(value)->privateName());
     371    ASSERT(value.isString());
     372    return asString(value)->toIdentifier(exec);
     373}
     374
     375void ScriptController::setupModuleScriptHandlers(LoadableModuleScript& moduleScriptRef, JSInternalPromise& promise, DOMWrapperWorld& world)
     376{
     377    auto& proxy = *windowProxy(world);
     378    auto& state = *proxy.window()->globalExec();
     379
     380    // It is not guaranteed that either fulfillHandler or rejectHandler is eventually called.
     381    // For example, if the page load is canceled, the DeferredPromise used in the module loader pipeline will stop executing JS code.
     382    // Thus the promise returned from this function could remain unresolved.
     383
     384    RefPtr<LoadableModuleScript> moduleScript(&moduleScriptRef);
     385
     386    auto& fulfillHandler = *JSNativeStdFunction::create(state.vm(), proxy.window(), 1, String(), [moduleScript](ExecState* exec) {
     387        Identifier moduleKey = jsValueToModuleKey(exec, exec->argument(0));
     388        moduleScript->notifyLoadCompleted(*moduleKey.impl());
     389        return JSValue::encode(jsUndefined());
     390    });
     391
     392    auto& rejectHandler = *JSNativeStdFunction::create(state.vm(), proxy.window(), 1, String(), [moduleScript](ExecState* exec) {
     393        VM& vm = exec->vm();
     394        JSValue errorValue = exec->argument(0);
     395        if (errorValue.isObject()) {
     396            auto* object = JSC::asObject(errorValue);
     397            if (JSValue failureKindValue = object->getDirect(vm, static_cast<JSVMClientData&>(*vm.clientData).builtinNames().failureKindPrivateName())) {
     398                // This is host propagated error in the module loader pipeline.
     399                switch (static_cast<ModuleFetchFailureKind>(failureKindValue.asInt32())) {
     400                case ModuleFetchFailureKind::WasErrored:
     401                    moduleScript->notifyLoadFailed(LoadableScript::Error {
     402                        LoadableScript::ErrorType::CachedScript,
     403                        std::nullopt
     404                    });
     405                    break;
     406                case ModuleFetchFailureKind::WasCanceled:
     407                    moduleScript->notifyLoadWasCanceled();
     408                    break;
     409                }
     410                return JSValue::encode(jsUndefined());
     411            }
     412        }
     413
     414        auto scope = DECLARE_CATCH_SCOPE(vm);
     415        moduleScript->notifyLoadFailed(LoadableScript::Error {
     416            LoadableScript::ErrorType::CachedScript,
     417            LoadableScript::ConsoleMessage {
     418                MessageSource::JS,
     419                MessageLevel::Error,
     420                retrieveErrorMessage(*exec, vm, errorValue, scope),
     421            }
     422        });
     423        return JSValue::encode(jsUndefined());
     424    });
     425
     426    promise.then(&state, &fulfillHandler, &rejectHandler);
     427}
     428
    363429TextPosition ScriptController::eventHandlerPosition() const
    364430{
  • trunk/Source/WebCore/bindings/js/ScriptController.h

    r223744 r223777  
    183183private:
    184184    WEBCORE_EXPORT JSDOMWindowProxy* initScript(DOMWrapperWorld&);
     185    void setupModuleScriptHandlers(LoadableModuleScript&, JSC::JSInternalPromise&, DOMWrapperWorld&);
    185186
    186187    void disconnectPlatformScriptObjects();
  • trunk/Source/WebCore/dom/LoadableModuleScript.cpp

    r223744 r223777  
    2929#include "Document.h"
    3030#include "Frame.h"
    31 #include "JSDOMExceptionHandling.h"
    32 #include "ModuleFetchFailureKind.h"
    3331#include "ModuleFetchParameters.h"
    3432#include "ScriptController.h"
    3533#include "ScriptElement.h"
    36 #include "WebCoreJSClientData.h"
    37 #include <heap/StrongInlines.h>
    38 #include <runtime/CatchScope.h>
    3934
    4035namespace WebCore {
     
    7570}
    7671
    77 void LoadableModuleScript::notifyLoadFailed(JSC::ExecState* exec, JSC::JSValue errorValue)
    78 {
    79     JSC::VM& vm = exec->vm();
    80     if (errorValue.isObject()) {
    81         auto* object = JSC::asObject(errorValue);
    82         if (JSC::JSValue failureKindValue = object->getDirect(vm, static_cast<JSVMClientData&>(*vm.clientData).builtinNames().failureKindPrivateName())) {
    83             // This is host propagated error in the module loader pipeline.
    84             switch (static_cast<ModuleFetchFailureKind>(failureKindValue.asInt32())) {
    85             case ModuleFetchFailureKind::WasErrored:
    86                 notifyLoadFailed(LoadableScript::Error {
    87                     LoadableScript::ErrorType::CachedScript,
    88                     std::nullopt
    89                 });
    90                 break;
    91             case ModuleFetchFailureKind::WasCanceled:
    92                 notifyLoadWasCanceled();
    93                 break;
    94             }
    95             return;
    96         }
    97     }
    98 
    99     auto scope = DECLARE_CATCH_SCOPE(vm);
    100     notifyLoadFailed(LoadableScript::Error {
    101         LoadableScript::ErrorType::CachedScript,
    102         LoadableScript::ConsoleMessage {
    103             MessageSource::JS,
    104             MessageLevel::Error,
    105             retrieveErrorMessage(*exec, vm, errorValue, scope),
    106         }
    107     });
    108 }
    109 
    11072void LoadableModuleScript::notifyLoadFailed(LoadableScript::Error&& error)
    11173{
  • trunk/Source/WebCore/dom/LoadableModuleScript.h

    r223744 r223777  
    5454    void load(Document&, const ScriptSourceCode&);
    5555
     56    void notifyLoadCompleted(UniquedStringImpl&);
     57    void notifyLoadFailed(LoadableScript::Error&&);
     58    void notifyLoadWasCanceled();
     59
    5660    UniquedStringImpl* moduleKey() const { return m_moduleKey.get(); }
    57 
    58     void notifyLoadCompleted(UniquedStringImpl&) final;
    59     void notifyLoadFailed(JSC::ExecState*, JSC::JSValue) final;
    6061
    6162private:
    6263    LoadableModuleScript(const String& nonce, const String& integrity, const String& crossOriginMode, const String& charset, const AtomicString& initiatorName, bool isInUserAgentShadowTree);
    63 
    64     void notifyLoadFailed(LoadableScript::Error&&);
    65     void notifyLoadWasCanceled();
    6664
    6765    Ref<ModuleFetchParameters> m_parameters;
Note: See TracChangeset for help on using the changeset viewer.