Bug 127648 – -[JSContext evaluteScript:] calls JSEvaluteScript with startingLineNumber 0, later interpreted as a oneBasedInt

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Joseph Pecoraro]

The inspector, via JSC::Debugger::scriptDidParse is seeing -[JSContext evaluteScript:] calls as scripts evaluated with an empty sourceURL starting at line -1. This can be tracked back to -[JSContext evaluteScript:] calling JSEvaluteScript with a 0 line number.

Seems like the JSEvaluteScript API should document that the lineNumber parameter is 1 based and we should update -[JSContext evaluteScript] to pass line 1 instead of 0. This would lead to better line numbers in the inspector when debugging a JSContext.

NOTE: There is also a FIXME in Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp that can be fixed:

    void InspectorDebuggerAgent::didParseSource(JSC::SourceID sourceID, const Script& inScript)
    {
        Script script = inScript;
        // FIXME: Why does -[JSContext evaluateScript] have a -1 startLine?
        if (script.startLine <= 0 && !script.startColumn)
            script.sourceURL = ContentSearchUtilities::findScriptSourceURL(script.source);
        ...
    }

Seeing as JSEvaluteScript is API and can be with any lineNumber, we might need to make the inspector (or the API) better support negative numbers.

[Joseph Pecoraro]

Although the documentation says these startingLineNumbers are "only used when reporting exceptions", now that we have the inspector the values can show in other places.

Lets clamp at all the API entry points. It never make sense to have a negative lineNumber.

[Joseph Pecoraro]

Ahh. I think I've worked this out.

In all the public JSC APIs with a startingLineNumber parameter it can only show up in JavaScript exceptions. And in those cases, the exception line number is correct, because ultimately the JSC::Source the APIs create does clamping of the line number appropriately. (See JSC::Source constructors and makeSource(...)).

However, for the same inputs, the JSC::SourceProvider created in the APIs has unclamped values. And it is these unclamped values that are passed to the inspector through JSC::Debugger::sourceParsed, and lead to the unexpected values seen by the inspector. As far as I can tell, JSC::SourceProvider's startPosition is _only_ used by the inspector.

I still think we should clamp at the API boundary.

[Joseph Pecoraro]

Created attachment 222384 [details]
[PATCH] Proposed Fix

[Geoffrey Garen]

Comment on attachment 222384 [details]
[PATCH] Proposed Fix

r=me

I'd remove "is interpreted", to be more direct, and hyphenate "one-based", since it's an adjective.

[Joseph Pecoraro]

(In reply to comment #4)
> (From update of attachment 222384 [details])
> r=me
> 
> I'd remove "is interpreted", to be more direct, and hyphenate "one-based", since it's an adjective.

Oo good suggestions. Will do.

[Joseph Pecoraro]

Landed <http://trac.webkit.org/changeset/162918> with rewording.