Web Inspector: Show Resource Initiator in Network Tab detail views

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Show Resource Initiator in Network Tab detail views

The Initiator information (such as a backtrace or location where it was requested) is only shown:

  • in the Network table (location link)
  • in the Resource Details Sidebar (location link)

But the full call frames are available and never used, and the Detail views don't include this info. We should show it somewhere.

[Radar WebKit Bug Importer]

<rdar://problem/49352679>

[Joseph Pecoraro]

Created attachment 366103 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 366104 [details]
[IMAGE] Dark

[Joseph Pecoraro]

Created attachment 366105 [details]
[IMAGE] Light

[Joseph Pecoraro]

I'd like to find some place to put the content that is not behind a click, but instead always visible. Perhaps an Initiator section near the bottom that always shows the stack. But for now I think this is a good idea.

It is possible that the use of the eye here conflates with the use of the eye in the Console. I think another icon we could use is a circle with 2 lines in it (=). I can try to implement such an icon.

[Joseph Pecoraro]

Created attachment 366110 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 366111 [details]
[IMAGE] Dark

[Joseph Pecoraro]

Created attachment 366112 [details]
[IMAGE] Light

[Devin Rousso]

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

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

r=me, nice!

Will be ready to land once the popover issue is resolved.

> Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js:309
> +            initiatorCallFrames: this._initiatorCallFramesFromPayload(initiator),

isn't this so much nicer than having to update every `new WI.Resource` :)

> Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js:776
> +        return callFrames.map((payload) => WI.CallFrame.fromPayload(WI.assumingMainTarget(), payload));

Should we be using `WI.StackTrace` instead?  That way, we "automatically" get support for async stack traces (if we decide to support that).  You'd have to mess with the `payload` a bit (`WI.StackTrace` expects a `callFrames` array in the payload), but it may be simpler (and provide more API options), such as possibly letting you eliminate the "custom" logic inside `_initiatorSourceCodeLocationFromPayload`.

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.css:62
> +    content: url(../Images/CallStack.svg);

NIT: I normally put `content` after and positional/sizing/spacing properties (e.g. `margin-left`).

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:124
> +    hidden()

Please call `super.hidden()`.

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:264
> +            const options = {

You could inline this.

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:276
> +                link.insertAdjacentElement("afterend", callStackElement);

Alternatively, you could create a `document.createDocumentFragment` and move the `this._summarySection.appendKeyValuePair` call to below the `if (callFrames) {`.
```
    let fragment = document.createDocumentFragment();

    let link = WI.createSourceCodeLocationLink(initiatorLocation, {
        dontFloat: true,
        ignoreSearchTab: true,
    });
    fragment.appendChild(link);

    let callFrames = this._resource.initiatorCallFrames;
    if (callFrames) {
        let callStackElement = fragment.appendChild(document.createElement("img"));
        ...
    }

    let pair = this._summarySection.appendKeyValuePair(WI.UIString("Initiator"), fragment);
    pair.classList.add("initiator");
```

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:282
> +                            let bounds = WI.Rect.rectFromClientRect(callStackElement.getBoundingClientRect());

Since this section can get "refreshed" (meaning we'd create a new `callStackElement` with a new "click" handler), you'd need to either save a reference to `this._callStackElement`, hide/reset `this._popover` each time the "click" handler is called, or update the `windowReziseHandler` each time the "click" event handler is called.

> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:298
> +                    let bounds = WI.Rect.rectFromClientRect(callStackElement.getBoundingClientRect());
> +                    this._popover.present(bounds.pad(2), [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);

Given that this is the same logic as `windowResizeHandler`, I'd pull both of them into a single function `presentBelowCallStackElement` and call it in each place.

[Joseph Pecoraro]

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

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

>> Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js:776
>> +        return callFrames.map((payload) => WI.CallFrame.fromPayload(WI.assumingMainTarget(), payload));
> 
> Should we be using `WI.StackTrace` instead?  That way, we "automatically" get support for async stack traces (if we decide to support that).  You'd have to mess with the `payload` a bit (`WI.StackTrace` expects a `callFrames` array in the payload), but it may be simpler (and provide more API options), such as possibly letting you eliminate the "custom" logic inside `_initiatorSourceCodeLocationFromPayload`.

If/when the backend sends a Console.StackTrace payload I'll use that, but for now it sends a list of Console.CallFrame payloads, so I'll stick with this for now.

>> Source/WebInspectorUI/UserInterface/Views/ResourceHeadersContentView.js:282
>> +                            let bounds = WI.Rect.rectFromClientRect(callStackElement.getBoundingClientRect());
> 
> Since this section can get "refreshed" (meaning we'd create a new `callStackElement` with a new "click" handler), you'd need to either save a reference to `this._callStackElement`, hide/reset `this._popover` each time the "click" handler is called, or update the `windowReziseHandler` each time the "click" event handler is called.

Good point.

[Joseph Pecoraro]

<https://trac.webkit.org/r243614>