Bug 214205 - Clean up SourceProvider and add caller relative load script to jsc.cpp
Summary: Clean up SourceProvider and add caller relative load script to jsc.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-10 15:23 PDT by Keith Miller
Modified: 2020-07-13 13:13 PDT (History)
13 users (show)

See Also:


Attachments
Patch (122.28 KB, patch)
2020-07-11 16:27 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (130.04 KB, patch)
2020-07-11 17:20 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (147.43 KB, patch)
2020-07-11 18:22 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (148.77 KB, patch)
2020-07-11 18:32 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (149.47 KB, patch)
2020-07-11 18:36 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (166.28 KB, patch)
2020-07-11 19:47 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch for landing (166.17 KB, patch)
2020-07-12 19:43 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch for landing (166.50 KB, patch)
2020-07-13 09:57 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2020-07-10 15:23:31 PDT
Clean up SourceProvider and add caller relative load script to jsc.cpp
Comment 1 Keith Miller 2020-07-11 16:27:32 PDT
Created attachment 404076 [details]
Patch
Comment 2 Keith Miller 2020-07-11 17:20:01 PDT
Created attachment 404077 [details]
Patch
Comment 3 Keith Miller 2020-07-11 18:22:17 PDT
Created attachment 404079 [details]
Patch
Comment 4 Keith Miller 2020-07-11 18:32:41 PDT
Created attachment 404080 [details]
Patch
Comment 5 Keith Miller 2020-07-11 18:36:57 PDT
Created attachment 404081 [details]
Patch
Comment 6 Keith Miller 2020-07-11 19:47:36 PDT
Created attachment 404083 [details]
Patch
Comment 7 Keith Miller 2020-07-11 19:52:33 PDT
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.
Comment 8 Yusuke Suzuki 2020-07-12 01:06:19 PDT
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.
Comment 9 Keith Miller 2020-07-12 19:43:10 PDT
Created attachment 404126 [details]
Patch for landing
Comment 10 EWS 2020-07-12 21:58:09 PDT
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
Comment 11 Keith Miller 2020-07-13 08:36:47 PDT
(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.
Comment 12 Keith Miller 2020-07-13 09:35:04 PDT
(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...
Comment 13 Keith Miller 2020-07-13 09:57:26 PDT
Created attachment 404152 [details]
Patch for landing
Comment 14 EWS 2020-07-13 10:35:41 PDT
Committed r264304: <https://trac.webkit.org/changeset/264304>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404152 [details].
Comment 15 Radar WebKit Bug Importer 2020-07-13 10:36:14 PDT
<rdar://problem/65485160>
Comment 16 Lauro Moura 2020-07-13 13:13:42 PDT
Commited new baseline for inspector/controller/runtime-controller-import.html in r264314 with the new message from resolveModuleSpecifier(...).