Bug 180996 – Web Inspector: Network Table - Redesign the waterfall popover showing timing data

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Network Table - Redesign the waterfall popover showing timing data

The current popover just shows a "big graph". Redesign the popover to be more about explanation.

[Joseph Pecoraro]

<rdar://problem/34072542>

[Joseph Pecoraro]

Created attachment 329810 [details]
[IMAGE] Waterfall Popover - All Fields

[Joseph Pecoraro]

Created attachment 329811 [details]
[IMAGE] Waterfall Popover - Some Fields

[Joseph Pecoraro]

Created attachment 329812 [details]
[IMAGE] Timing Detail View - Most Fields

[Joseph Pecoraro]

Created attachment 329813 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

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

Oops, need to rebase.

[Joseph Pecoraro]

Created attachment 329817 [details]
[PATCH] Proposed Fix

[EWS Watchlist]

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

Attachment 329817 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5760511

New failing tests:
webrtc/video-replace-muted-track.html

[EWS Watchlist]

Created attachment 329845 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

Comment on attachment 329845 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

Failure appears to be unrelated to the patch.

[Matt Baker]

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

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

r=me, with a couple nits/comments.

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingBreakdownView.css:32
> +    color: gray;

Style: every use of `gray` could be replaced with var(--text-color-gray-medium), but I'll leave it up to you. We should probably just update the variable's value to be gray instead of hsl(0, 0%, 52%).

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingBreakdownView.css:41
> +    padding-left: 5px;

Use -webkit-padding-start for this to be RTL ready.

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingBreakdownView.js:33
> +        console.assert(!fixedWidth || fixedWidth >= 100, "fixedWidth must be at least wide enough for strings");

Nit: assert description should end in a period.

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingBreakdownView.js:56
> +        if (additionalClassName)

I was about to suggest row.classList.add("header", additionalClassName), but learned that `null` and `undefined` get converted to "null" and "undefined" and added as classes!

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingContentView.js:97
> +        breakdownView.updateLayout();

The breakdownView isn't being added to the view hierarchy. This is okay, so long as it never needs to layout again (which seems to be the case, as it implements View.initialLayout but not View.layout). The call to updateLayout does the trick just fine here (a scheduled layout would never run), but thought it was worth mentioning.

Earlier ResourceTimingBreakdownView is used in as popover, which isn't a part of the View hierarchy currently, so updateLayout is required. That said, eventually we should make Popover a view that is a child of the root (document.body).

[Joseph Pecoraro]

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