Bug 156022 - Web Inspector: sourceMappingURL not loaded in generated script
Summary: Web Inspector: sourceMappingURL not loaded in generated script
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac OS X 10.11
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 156863
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-30 05:22 PDT by eelco
Modified: 2016-04-21 17:49 PDT (History)
13 users (show)

See Also:


Attachments
Example to reproduce the bug (39.69 KB, application/zip)
2016-03-30 05:22 PDT, eelco
no flags Details
[PATCH] Proposed Fix (28.70 KB, patch)
2016-04-21 15:43 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[IMAGE] After - Test Case (89.93 KB, image/png)
2016-04-21 15:43 PDT, Joseph Pecoraro
no flags Details
[IMAGE] After - Eval Code console messages with/without sourceURL name (131.44 KB, image/png)
2016-04-21 15:43 PDT, Joseph Pecoraro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description eelco 2016-03-30 05:22:16 PDT
Created attachment 275194 [details]
Example to reproduce the bug

The source map from a generated script is not loaded.

To reproduce:
- Unzip the attachment.
- Host it by executing 'python -m SimpleHTTPServer 1234'
- Go to http://0.0.0.0:1234
- Open the Web Inspector Console (option-cmd-C)
- Look at the first error

Observed:
- The error is reported as originating from the function `anonymous`, not linked to any source.

Expected:
- The error is reported as originating from app.coffee:1 or at the very least from the 'Extra Script' coffeescript:3

Notes:
This might be a duplicate of https://bugs.webkit.org/show_bug.cgi?id=156021
Comment 1 Radar WebKit Bug Importer 2016-03-30 05:22:38 PDT
<rdar://problem/25438595>
Comment 2 Joseph Pecoraro 2016-04-14 19:40:16 PDT
This is another great reduction!

Even after fixing bug 156021, this exposes another issue. We aren't reliably detecting the sourceURL within functions created via `Function`.

* TEST
<script>
console.log(Function("\n//# sourceURL=test.js\nreturn 1+1")());
</script>

I'd expect this to always create a Extra Script named "test.js" however it only does it rarely (once or twice) and then never again.
Comment 3 Joseph Pecoraro 2016-04-20 16:29:09 PDT
Hmm, the so issue here is the sourceURL/sourceMappingURL do not get added when we skip re-parsing via the CodeCache:

When constructing the executable for `Function(str)` we go through:

    frame #0: JSC::CodeCache::getFunctionExecutableFromGlobalCode
    frame #1: JSC::UnlinkedFunctionExecutable::fromGlobalCode
    frame #2: JSC::FunctionExecutable::fromGlobalCode
    frame #3: JSC::constructFunctionSkippingEvalEnabledCheck
    frame #4: JSC::constructFunction
    frame #5: JSC::constructFunction
    frame #6: JSC::callFunctionConstructor

At which point if we do not have it in the CodeCache we parse it. If we do have this in the code cache, we return the cached unlinked function executable skipping parsing.

By skipping parsing we don't update the SourceProvider with the sourceURL/sourceMappingURL, because we normally do that under JSC::Parser<>::parse.
Comment 4 Joseph Pecoraro 2016-04-21 13:23:07 PDT
Fixing the CodeCache issue then revealed a few more issues.

  1. Using CodeCache and skipping loses source directives (Function constructor)
  2. Frontend didn't properly handle a top level sourceMappingURL being a dataURL
  3. Exceptions thrown in non-resource scripts would not have resource links in
     the console, because we couldn't map them back to a resource. (no url)

Fixing all of these gets this working. I'm going to first fix the CodeCache issue separately, because I think the JavaScriptCore folks may have some input on that.
Comment 5 Joseph Pecoraro 2016-04-21 15:43:03 PDT
Created attachment 276973 [details]
[PATCH] Proposed Fix
Comment 6 Joseph Pecoraro 2016-04-21 15:43:27 PDT
Created attachment 276974 [details]
[IMAGE] After - Test Case
Comment 7 Joseph Pecoraro 2016-04-21 15:43:55 PDT
Created attachment 276975 [details]
[IMAGE] After - Eval Code console messages with/without sourceURL name
Comment 8 WebKit Commit Bot 2016-04-21 15:44:42 PDT
Attachment 276973 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/ScriptCallFrame.h:44:  The parameter name "sourceID" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Geoffrey Garen 2016-04-21 16:58:57 PDT
Comment on attachment 276973 [details]
[PATCH] Proposed Fix

r=me
Comment 10 WebKit Commit Bot 2016-04-21 17:49:49 PDT
Comment on attachment 276973 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 276973

Committed r199852: <http://trac.webkit.org/changeset/199852>
Comment 11 WebKit Commit Bot 2016-04-21 17:49:54 PDT
All reviewed patches have been landed.  Closing bug.