Bug 214205 – Clean up SourceProvider and add caller relative load script to jsc.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

Clean up SourceProvider and add caller relative load script to jsc.cpp

[Keith Miller]

Created attachment 404076 [details]
Patch

[Keith Miller]

Created attachment 404077 [details]
Patch

[Keith Miller]

Created attachment 404079 [details]
Patch

[Keith Miller]

Created attachment 404080 [details]
Patch

[Keith Miller]

Created attachment 404081 [details]
Patch

[Keith Miller]

Created attachment 404083 [details]
Patch

[Keith Miller]

There are some Layout test failures for now because I don't know how to handle system paths in the expectations. I'll figure that out before landing.

[Yusuke Suzuki]

Comment on attachment 404083 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404083&action=review

r=me

> Source/JavaScriptCore/ChangeLog:268
> +        * API/JSAPIGlobalObject.mm:
> +        (JSC::JSAPIGlobalObject::moduleLoaderImportModule):
> +        * API/JSBase.cpp:
> +        (JSEvaluateScript):
> +        (JSCheckScriptSyntax):
> +        * API/JSObjectRef.cpp:
> +        (JSObjectMakeFunction):
> +        * API/JSScript.mm:
> +        (-[JSScript sourceCode]):
> +        * API/JSScriptRef.cpp:
> +        * builtins/BuiltinExecutables.cpp:
> +        (JSC::BuiltinExecutables::BuiltinExecutables):
> +        * bytecode/CodeBlock.h:
> +        (JSC::CodeBlock::referrerURL const):
> +        * debugger/DebuggerLocation.cpp:
> +        (JSC::DebuggerLocation::DebuggerLocation):
> +        * debugger/DebuggerLocation.h:
> +        (JSC::DebuggerLocation::DebuggerLocation):
> +        * inspector/ScriptDebugServer.cpp:
> +        (Inspector::ScriptDebugServer::sourceParsed):
> +        * jsc.cpp:
> +        (GlobalObject::moduleLoaderImportModule):
> +        (jscSource):
> +        (GlobalObject::moduleLoaderFetch):
> +        (functionRun):
> +        (functionLoad):
> +        (functionCheckSyntax):
> +        (functionCallerSourceOrigin):
> +        (functionDollarAgentStart):
> +        (functionCheckModuleSyntax):
> +        (runWithOptions):
> +        (runInteractive):
> +        * parser/Nodes.h:
> +        (JSC::ScopeNode::source const):
> +        (JSC::ScopeNode::sourceURL const): Deleted.
> +        * parser/SourceCode.h:
> +        (JSC::makeSource):
> +        * parser/SourceCodeKey.h:
> +        (JSC::SourceCodeKey::host const):
> +        * parser/SourceProvider.cpp:
> +        (JSC::SourceProvider::SourceProvider):
> +        * parser/SourceProvider.h:
> +        (JSC::SourceProvider::sourceURL const):
> +        (JSC::StringSourceProvider::create):
> +        (JSC::StringSourceProvider::StringSourceProvider):
> +        (JSC::SourceProvider::url const): Deleted.
> +        * runtime/CachedTypes.cpp:
> +        (JSC::CachedSourceOrigin::encode):
> +        (JSC::CachedSourceOrigin::decode const):
> +        (JSC::CachedSourceProviderShape::encode):
> +        (JSC::CachedStringSourceProvider::decode const):
> +        (JSC::CachedWebAssemblySourceProvider::decode const):
> +        * runtime/Error.cpp:
> +        (JSC::addErrorInfo):
> +        * runtime/FunctionConstructor.cpp:
> +        (JSC::constructFunctionSkippingEvalEnabledCheck):
> +        * runtime/ScriptExecutable.h:
> +        (JSC::ScriptExecutable::sourceURL const):
> +        * runtime/SourceOrigin.h:
> +        (JSC::SourceOrigin::SourceOrigin):
> +        (JSC::SourceOrigin::url const):
> +        (JSC::SourceOrigin::string const):
> +        (JSC::SourceOrigin::isNull const): Deleted.
> +        * runtime/ThrowScope.cpp:
> +        (JSC::ThrowScope::throwException):
> +        * runtime/ThrowScope.h:
> +        (JSC::ThrowScope::throwException):
> +        (JSC::throwVMException):
> +        * tools/FunctionOverrides.cpp:
> +        (JSC::initializeOverrideInfo):
> +
> +        * API/JSAPIGlobalObject.mm:
> +        (JSC::computeValidImportSpecifier):
> +        (JSC::JSAPIGlobalObject::moduleLoaderImportModule):
> +        * API/JSBase.cpp:
> +        (JSEvaluateScript):
> +        (JSCheckScriptSyntax):
> +        * API/JSObjectRef.cpp:
> +        (JSObjectMakeFunction):
> +        * API/JSScript.mm:
> +        (-[JSScript sourceCode]):
> +        * API/JSScriptRef.cpp:
> +        * API/glib/JSCContext.cpp:
> +        (jsc_context_check_syntax):
> +        * builtins/BuiltinExecutables.cpp:
> +        (JSC::BuiltinExecutables::BuiltinExecutables):
> +        * debugger/DebuggerLocation.cpp:
> +        (JSC::DebuggerLocation::DebuggerLocation):
> +        * debugger/DebuggerLocation.h:
> +        (JSC::DebuggerLocation::DebuggerLocation):
> +        * inspector/ScriptDebugServer.cpp:
> +        (Inspector::ScriptDebugServer::sourceParsed):
> +        * jsc.cpp:
> +        (currentWorkingDirectory):
> +        (absolutePath):
> +        (GlobalObject::moduleLoaderImportModule):
> +        (GlobalObject::moduleLoaderResolve):
> +        (jscSource):
> +        (fetchModuleFromLocalFileSystem):
> +        (GlobalObject::moduleLoaderFetch):
> +        (functionLoad):
> +        (functionCallerSourceOrigin):
> +        (functionDollarAgentStart):
> +        (functionCheckModuleSyntax):
> +        (runWithOptions):
> +        (runInteractive):
> +        (ModuleName::startsWithRoot const): Deleted.
> +        (ModuleName::ModuleName): Deleted.
> +        (extractDirectoryName): Deleted.
> +        (resolvePath): Deleted.
> +        * parser/Nodes.h:
> +        (JSC::ScopeNode::source const):
> +        (JSC::ScopeNode::sourceURL const): Deleted.
> +        * parser/SourceCode.h:
> +        (JSC::makeSource):
> +        * parser/SourceCodeKey.h:
> +        (JSC::SourceCodeKey::host const):
> +        * parser/SourceProvider.cpp:
> +        (JSC::SourceProvider::SourceProvider):
> +        * parser/SourceProvider.h:
> +        (JSC::SourceProvider::sourceURL const):
> +        (JSC::StringSourceProvider::create):
> +        (JSC::StringSourceProvider::StringSourceProvider):
> +        (JSC::SourceProvider::url const): Deleted.
> +        * runtime/CachedTypes.cpp:
> +        (JSC::CachedSourceOrigin::encode):
> +        (JSC::CachedSourceOrigin::decode const):
> +        (JSC::CachedSourceProviderShape::encode):
> +        (JSC::CachedStringSourceProvider::decode const):
> +        (JSC::CachedWebAssemblySourceProvider::decode const):
> +        * runtime/Error.cpp:
> +        (JSC::addErrorInfo):
> +        * runtime/FunctionConstructor.cpp:
> +        (JSC::constructFunctionSkippingEvalEnabledCheck):
> +        * runtime/ScriptExecutable.h:
> +        (JSC::ScriptExecutable::sourceURL const):
> +        * runtime/SourceOrigin.h:
> +        (JSC::SourceOrigin::SourceOrigin):
> +        (JSC::SourceOrigin::url const):
> +        (JSC::SourceOrigin::string const):
> +        (JSC::SourceOrigin::isNull const):
> +        * runtime/ThrowScope.cpp:
> +        (JSC::ThrowScope::throwException):
> +        * runtime/ThrowScope.h:
> +        (JSC::ThrowScope::throwException):
> +        (JSC::throwVMException):
> +        * tools/FunctionOverrides.cpp:
> +        (JSC::initializeOverrideInfo):
> +        * tools/JSDollarVM.cpp:
> +        (JSC::doPrint):
> +        (JSC::functionCrash):
> +
> +        * API/JSAPIGlobalObject.mm:
> +        (JSC::computeValidImportSpecifier):
> +        (JSC::JSAPIGlobalObject::moduleLoaderImportModule):
> +        * API/JSBase.cpp:
> +        (JSEvaluateScript):
> +        (JSCheckScriptSyntax):
> +        * API/JSObjectRef.cpp:
> +        (JSObjectMakeFunction):
> +        * API/JSScript.mm:
> +        (-[JSScript sourceCode]):
> +        * API/JSScriptRef.cpp:
> +        * API/glib/JSCContext.cpp:
> +        (jsc_context_check_syntax):
> +        * builtins/BuiltinExecutables.cpp:
> +        (JSC::BuiltinExecutables::BuiltinExecutables):
> +        * debugger/DebuggerLocation.cpp:
> +        (JSC::DebuggerLocation::DebuggerLocation):
> +        * debugger/DebuggerLocation.h:
> +        (JSC::DebuggerLocation::DebuggerLocation):
> +        * inspector/ScriptDebugServer.cpp:
> +        (Inspector::ScriptDebugServer::sourceParsed):
> +        * jsc.cpp:
> +        (currentWorkingDirectory):
> +        (absolutePath):
> +        (GlobalObject::moduleLoaderImportModule):
> +        (GlobalObject::moduleLoaderResolve):
> +        (jscSource):
> +        (fetchModuleFromLocalFileSystem):
> +        (GlobalObject::moduleLoaderFetch):
> +        (functionLoad):
> +        (functionCallerSourceOrigin):
> +        (functionDollarAgentStart):
> +        (functionCheckModuleSyntax):
> +        (runWithOptions):
> +        (runInteractive):
> +        (ModuleName::startsWithRoot const): Deleted.
> +        (ModuleName::ModuleName): Deleted.
> +        (extractDirectoryName): Deleted.
> +        (resolvePath): Deleted.
> +        * parser/Nodes.h:
> +        (JSC::ScopeNode::source const):
> +        (JSC::ScopeNode::sourceURL const): Deleted.
> +        * parser/SourceCode.h:
> +        (JSC::makeSource):
> +        * parser/SourceCodeKey.h:
> +        (JSC::SourceCodeKey::host const):
> +        * parser/SourceProvider.cpp:
> +        (JSC::SourceProvider::SourceProvider):
> +        * parser/SourceProvider.h:
> +        (JSC::SourceProvider::sourceURL const):
> +        (JSC::StringSourceProvider::create):
> +        (JSC::StringSourceProvider::StringSourceProvider):
> +        (JSC::SourceProvider::url const): Deleted.
> +        * runtime/CachedTypes.cpp:
> +        (JSC::CachedSourceOrigin::encode):
> +        (JSC::CachedSourceOrigin::decode const):
> +        (JSC::CachedSourceProviderShape::encode):
> +        (JSC::CachedStringSourceProvider::decode const):
> +        (JSC::CachedWebAssemblySourceProvider::decode const):
> +        * runtime/Error.cpp:
> +        (JSC::addErrorInfo):
> +        * runtime/FunctionConstructor.cpp:
> +        (JSC::constructFunctionSkippingEvalEnabledCheck):
> +        * runtime/ScriptExecutable.h:
> +        (JSC::ScriptExecutable::sourceURL const):
> +        * runtime/SourceOrigin.h:
> +        (JSC::SourceOrigin::SourceOrigin):
> +        (JSC::SourceOrigin::url const):
> +        (JSC::SourceOrigin::string const):
> +        (JSC::SourceOrigin::isNull const):
> +        * runtime/ThrowScope.cpp:
> +        (JSC::ThrowScope::throwException):
> +        * runtime/ThrowScope.h:
> +        (JSC::ThrowScope::throwException):
> +        (JSC::throwVMException):
> +        * tools/FunctionOverrides.cpp:
> +        (JSC::initializeOverrideInfo):
> +        * tools/JSDollarVM.cpp:
> +        (JSC::doPrint):
> +        (JSC::functionCrash):

Let's remove this.

> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:153
> +    URL baseURL = sourceOrigin.url() ? sourceOrigin.url() : URL();

Is this condition necessary? Why not using `URL baseURL = sourceOrigin.url();`?

> LayoutTests/ChangeLog:2
> +        Clean up SourceProvider and add caller relative load script to jsc.cpp

Need space above.

[Keith Miller]

Created attachment 404126 [details]
Patch for landing

[EWS]

Found 10 new test failures: fast/dom/Document/replace-child.html, fast/visual-viewport/nonzoomed-rects.html, fast/visual-viewport/rtl-nonzoomed-rects.html, fast/visual-viewport/rtl-zoomed-rects.html, fast/visual-viewport/rubberbanding-viewport-rects-extended-background.html, fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html, fast/visual-viewport/rubberbanding-viewport-rects.html, fast/visual-viewport/zoomed-fixed-header-and-footer.html, fast/visual-viewport/zoomed-fixed.html, fast/visual-viewport/zoomed-rects.html

[Keith Miller]

(In reply to EWS from comment #10)
> Found 10 new test failures: fast/dom/Document/replace-child.html,
> fast/visual-viewport/nonzoomed-rects.html,
> fast/visual-viewport/rtl-nonzoomed-rects.html,
> fast/visual-viewport/rtl-zoomed-rects.html,
> fast/visual-viewport/rubberbanding-viewport-rects-extended-background.html,
> fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html,
> fast/visual-viewport/rubberbanding-viewport-rects.html,
> fast/visual-viewport/zoomed-fixed-header-and-footer.html,
> fast/visual-viewport/zoomed-fixed.html,
> fast/visual-viewport/zoomed-rects.html

Looks like something went wrong on that test run. It doesn’t seem related to my patch.

[Keith Miller]

(In reply to Keith Miller from comment #11)
> (In reply to EWS from comment #10)
> > Found 10 new test failures: fast/dom/Document/replace-child.html,
> > fast/visual-viewport/nonzoomed-rects.html,
> > fast/visual-viewport/rtl-nonzoomed-rects.html,
> > fast/visual-viewport/rtl-zoomed-rects.html,
> > fast/visual-viewport/rubberbanding-viewport-rects-extended-background.html,
> > fast/visual-viewport/rubberbanding-viewport-rects-header-footer.html,
> > fast/visual-viewport/rubberbanding-viewport-rects.html,
> > fast/visual-viewport/zoomed-fixed-header-and-footer.html,
> > fast/visual-viewport/zoomed-fixed.html,
> > fast/visual-viewport/zoomed-rects.html
> 
> Looks like something went wrong on that test run. It doesn’t seem related to
> my patch.

Oh weird, it looks like this was my patch. I don't know why it didn't fail the first run...

[Keith Miller]

Created attachment 404152 [details]
Patch for landing

[EWS]

Committed r264304: <https://trac.webkit.org/changeset/264304>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404152 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65485160>

[Lauro Moura]

Commited new baseline for inspector/controller/runtime-controller-import.html in r264314 with the new message from resolveModuleSpecifier(...).