[Webkit-unassigned] [Bug 190440] WebKit Inspector: Expose Server Timing Response Headers in Network Tab

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=190440

Devin Rousso <drousso at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |drousso at apple.com

--- Comment #28 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 352214
  --> https://bugs.webkit.org/attachment.cgi?id=352214
Patch

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

This is awesome!  One thing to note is that you're missing a ChangeLog for the LayoutTests changes :(

> Source/WebInspectorUI/UserInterface/Models/ServerTimingEntry.js:180
> +        if (!this._durationSet) {

Is it necessary to prevent modification of this value once it's set?  Typically, our style would be to `console.assert` for this, as it's more of a programmer's responsibility to control this.

> Source/WebInspectorUI/UserInterface/Models/ServerTimingEntry.js:188
> +        if (!this._descriptionSet) {

Ditto (180).

> Source/WebInspectorUI/UserInterface/Views/ResourceTimingBreakdownView.js:140
> +        let {

I would prefer if you separated this onto two lines.  The reason we destructured `timingData` is because it's just a plain object, whereas `timingData` is an actual getter.  I don't think we should be destructuring our model objects.

    let {startTime, redirectStart, redirectEnd, fetchStart, domainLookupStart, domainLookupEnd, connectStart, connectEnd, secureConnectionStart, requestStart, responseStart, responseEnd} = this._resource.timingData;
    let serverTiming = this._resource.serverTiming;

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181012/ff507f34/attachment-0001.html>