Bug 67205 - Web Inspector: implement source map v3 consumer.
Summary: Web Inspector: implement source map v3 consumer.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Podivilov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-30 09:23 PDT by Pavel Podivilov
Modified: 2011-09-06 10:35 PDT (History)
10 users (show)

See Also:


Attachments
Patch (14.74 KB, patch)
2011-08-30 09:24 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (14.85 KB, patch)
2011-09-05 10:38 PDT, Pavel Podivilov
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2011-08-30 09:23:21 PDT
Web Inspector: implement source map v3 consumer.
Comment 1 Pavel Podivilov 2011-08-30 09:24:56 PDT
Created attachment 105640 [details]
Patch
Comment 3 Yury Semikhatsky 2011-09-05 02:37:35 PDT
Comment on attachment 105640 [details]
Patch

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

> Source/WebCore/inspector/front-end/CompilerSourceMapping.js:49
> +    get sources()

This should be a function since it may be overridden.

> Source/WebCore/inspector/front-end/CompilerSourceMapping.js:59
> +WebInspector.ClosureCompilerSourceMapping = function(payload)

Is there a document describing the mapping format? If so, please add a link to the document here, otherwise provide a description of the format.

> Source/WebCore/inspector/front-end/CompilerSourceMapping.js:101
> +              count -= step;

Can we replace count with step in the loop and get rid of these "count =" assignments?
Comment 4 Pavel Podivilov 2011-09-05 10:38:40 PDT
Created attachment 106346 [details]
Patch
Comment 5 Pavel Podivilov 2011-09-05 10:40:50 PDT
(In reply to comment #3)
> (From update of attachment 105640 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105640&action=review
> 
> > Source/WebCore/inspector/front-end/CompilerSourceMapping.js:49
> > +    get sources()
> 
> This should be a function since it may be overridden.

I think getters/setters can also be overridden just like regular functions.

> 
> > Source/WebCore/inspector/front-end/CompilerSourceMapping.js:59
> > +WebInspector.ClosureCompilerSourceMapping = function(payload)
> 
> Is there a document describing the mapping format? If so, please add a link to the document here, otherwise provide a description of the format.

Done.

> 
> > Source/WebCore/inspector/front-end/CompilerSourceMapping.js:101
> > +              count -= step;
> 
> Can we replace count with step in the loop and get rid of these "count =" assignments?

I don't think so, both count and step are required here since count could be odd.
Comment 6 Yury Semikhatsky 2011-09-05 23:40:59 PDT
Comment on attachment 106346 [details]
Patch

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

> Source/WebCore/inspector/front-end/CompilerSourceMapping.js:61
> +    // This class implements Source Map V3 consumer. Source Map V3 format is described

The comment should go into the constructor description, not in its body.
Comment 7 Pavel Podivilov 2011-09-06 10:35:45 PDT
Committed r94577: <http://trac.webkit.org/changeset/94577>