[webkit-reviews] review granted: [Bug 214205] Clean up SourceProvider and add caller relative load script to jsc.cpp : [Attachment 404083] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 214205: Clean up SourceProvider and add caller relative load script to
jsc.cpp
https://bugs.webkit.org/show_bug.cgi?id=214205

Attachment 404083: Patch

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




--- Comment #8 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 404083
  --> https://bugs.webkit.org/attachment.cgi?id=404083
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.