Bug 32610 – Web Inspector: Links to source in CPU profiles should open Scripts (debugger) panel

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Mikhail Naganov]

It is more convenient to explore performance issues using debugger, not resources panel.

[Mikhail Naganov]

Created attachment 44973 [details]
Proposed fix

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44973 [details] without any errors.

[Pavel Feldman]

Comment on attachment 44973 [details]
Proposed fix

>  WebInspector.canShowResourceForURL = function(url, preferredPanel)
>  WebInspector.showResourceForURL = function(url, line, preferredPanel)

It would be much more clear if you made Panel's canShowResource and showResource accept URL instead of resource. That way you would be able to make canShowResourceForURL and showResourceForURL here panel-agnostic.

[Mikhail Naganov]

Created attachment 45012 [details]
proposed change, comments addressed

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45012 [details] without any errors.

[Pavel Feldman]

Comment on attachment 45012 [details]
proposed change, comments addressed

> +WebInspector._choosePanelToShowResourceForURL = function(url, preferredPanel)
>  {
> -    var resource = this.resourceForURL(url);
> -    if (!resource)
> -        return false;
> -
> +    var panel = null;
>      if (preferredPanel && preferredPanel in WebInspector.panels) {
> -        var panel = this.panels[preferredPanel];
> -        if (!("showResource" in panel))
> +        panel = this.panels[preferredPanel];
> +        if (!("showResourceForURL" in panel))
>              panel = null;
> -        else if ("canShowResource" in panel && !panel.canShowResource(resource))
> +        else if ("canShowResourceForURL" in panel && !panel.canShowResourceForURL(url))
>              panel = null;
>      }
> +    if (!panel && preferredPanel != "resources")
> +        return this._choosePanelToShowResourceForURL(url, "resources");
> +    return panel;
> +}

Can we implement this a bit more simple?
- Require that any preferredPanel has necessary canShow and show methods
- first check against preferredPanel, in case of failure explicitly check against resources. No need to reenter method.

> +
> +WebInspector.showResourceForURL = function(url, line, preferredPanel)
> +{

This name no longer applies. WebInspector.showScriptOrResourceForURL ?

r- for confusing logic in _choosePanelToShowResourceForURL and name of the method.

[Mikhail Naganov]

Created attachment 45060 [details]
Proposed fix, comments addressed

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45060 [details] without any errors.

[Pavel Feldman]

Comment on attachment 45060 [details]
Proposed fix, comments addressed

> +    showSourceLineForURL: function(url, line)
> +    {
> +        this.showResource(WebInspector.resourceForURL(url));

You lose line information here. Rest is Ok.

[Mikhail Naganov]

Created attachment 45064 [details]
fixed 'line' argument loss for resources panel

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45064 [details] without any errors.

[WebKit Commit Bot]

Comment on attachment 45064 [details]
fixed 'line' argument loss for resources panel

Clearing flags on attachment: 45064

Committed r52253: <http://trac.webkit.org/changeset/52253>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.