Bug 65612 - Web Inspector: draft implementation of source mapping listeners.
: Web Inspector: draft implementation of source mapping listeners.
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Pavel Podivilov
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-03 05:31 PDT by Pavel Podivilov
Modified: 2011-08-08 08:51 PDT (History)
12 users (show)

See Also:


Attachments
Patch (13.08 KB, patch)
2011-08-03 05:32 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (14.06 KB, patch)
2011-08-03 07:19 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (25.16 KB, patch)
2011-08-04 06:04 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (26.55 KB, patch)
2011-08-04 06:46 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (26.55 KB, patch)
2011-08-04 07:07 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Patch (26.55 KB, patch)
2011-08-04 07:08 PDT, Pavel Podivilov
pfeldman: 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-03 05:31:58 PDT
Web Inspector: draft implementation of source mapping listeners.

Refactor anchors creation: move anchor updating code to ScriptsPanel since we have model.addSourceMappingListener now.
Comment 1 Pavel Podivilov 2011-08-03 05:32:57 PDT
Created attachment 102768 [details]
Patch
Comment 2 WebKit Review Bot 2011-08-03 05:56:18 PDT
Comment on attachment 102768 [details]
Patch

Attachment 102768 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9304015

New failing tests:
http/tests/inspector-enabled/console-clear-arguments-on-frame-remove.html
Comment 3 Pavel Podivilov 2011-08-03 07:19:05 PDT
Created attachment 102783 [details]
Patch
Comment 4 Pavel Feldman 2011-08-04 03:08:39 PDT
Comment on attachment 102783 [details]
Patch

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

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:125
> +        this._sourceMappingListeners.push(listener);

Nit: You could do addEventListener(sourceId, listener) followed by a dispatchEvent(this.sourceId)...

> Source/WebCore/inspector/front-end/ScriptsPanel.js:572
> +            model.addSourceMappingListener(url, null, formatAnchor);

Could we test anchor update?

> Source/WebCore/inspector/front-end/inspector.js:1323
> +WebInspector._linkifyLocation = function(url, oneBasedLineNumber, oneBasedColumnNumber, classes, tooltipText, preferredPanel)

Call sites should probably do WebInspector.debuggerPresentationModel.linkifyLocation(location), where location is raw 'script' location.

> Source/WebCore/inspector/front-end/inspector.js:1325
> +    if (preferredPanel === "scripts" && !WebInspector.debuggerModel.scriptsForURL(url).length)

Either this code or WebInspector._showAnchorLocation should fall back to "resources".
Comment 5 Pavel Podivilov 2011-08-04 06:04:02 PDT
Created attachment 102902 [details]
Patch
Comment 6 Pavel Podivilov 2011-08-04 06:11:21 PDT
(In reply to comment #4)
> (From update of attachment 102783 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102783&action=review
> 
> > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:125
> > +        this._sourceMappingListeners.push(listener);
> 
> Nit: You could do addEventListener(sourceId, listener) followed by a dispatchEvent(this.sourceId)...

Unfortunately, sourceId isn't a good id since there may be several scripts linked to one RawSourceCode instance.

> 
> > Source/WebCore/inspector/front-end/ScriptsPanel.js:572
> > +            model.addSourceMappingListener(url, null, formatAnchor);
> 
> Could we test anchor update?

It is tested by testConsoleMessagesForFormattedScripts in script-formatter.html.

> 
> > Source/WebCore/inspector/front-end/inspector.js:1323
> > +WebInspector._linkifyLocation = function(url, oneBasedLineNumber, oneBasedColumnNumber, classes, tooltipText, preferredPanel)
> 
> Call sites should probably do WebInspector.debuggerPresentationModel.linkifyLocation(location), where location is raw 'script' location.

Done.

> 
> > Source/WebCore/inspector/front-end/inspector.js:1325
> > +    if (preferredPanel === "scripts" && !WebInspector.debuggerModel.scriptsForURL(url).length)
> 
> Either this code or WebInspector._showAnchorLocation should fall back to "resources".

Not all anchors are created by linkifyResourceAsNode, some of them don't have preferred_panel attribute. Code in WebInspector._showAnchorLocation is needed for such anchors.
Comment 7 WebKit Review Bot 2011-08-04 06:27:30 PDT
Comment on attachment 102902 [details]
Patch

Attachment 102902 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9303457

New failing tests:
http/tests/inspector/console-resource-errors.html
http/tests/inspector/console-xhr-logging.html
Comment 8 Pavel Podivilov 2011-08-04 06:46:58 PDT
Created attachment 102905 [details]
Patch
Comment 9 Pavel Podivilov 2011-08-04 07:07:31 PDT
Created attachment 102909 [details]
Patch
Comment 10 Pavel Podivilov 2011-08-04 07:08:30 PDT
Created attachment 102910 [details]
Patch
Comment 11 Pavel Feldman 2011-08-05 08:47:35 PDT
Comment on attachment 102910 [details]
Patch

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

> Source/WebCore/inspector/front-end/EventListenersSidebarPane.js:240
> +            var url = this.eventListener.location.scriptId;

Please add yet another FIXME on the location structure + use of column.

> Source/WebCore/inspector/front-end/ProfileDataGridTree.js:101
> +            var urlElement = WebInspector.debuggerPresentationModel.linkifyLocation(this.profileNode.url, lineNumber, 0, "profile-node-file");

Do you know what url is?

> Source/WebCore/inspector/front-end/TimelinePanel.js:1128
> +                    return this._linkifyLocation(this.stackTrace[0].url, this.stackTrace[0].lineNumber, this.stackTrace[0].columnNumber);

Extract method receiving ConsoleCallFrame
Comment 12 Pavel Podivilov 2011-08-08 08:51:03 PDT
Committed r92598: <http://trac.webkit.org/changeset/92598>