[webkit-reviews] review granted: [Bug 189773] Web Inspector: create special Network waterfall for media events : [Attachment 351856] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Oct 10 15:51:56 PDT 2018
Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 189773: Web Inspector: create special Network waterfall for media events
https://bugs.webkit.org/show_bug.cgi?id=189773
Attachment 351856: Patch
https://bugs.webkit.org/attachment.cgi?id=351856&action=review
--- Comment #13 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 351856
--> https://bugs.webkit.org/attachment.cgi?id=351856
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=351856&action=review
r=me
> Source/WebInspectorUI/UserInterface/Base/Utilities.js:467
> +Object.defineProperty(Array.prototype, "adjacentForEach",
I still don't see value in this. If I read this I'd think it would run callback
with both the adjacent elements, something like (before, current, next)
( undefi, arr[0], arr[1] )
( arr[0], arr[1], arr[2] )
( arr[1], arr[2], arr[3] )
( arr[2], arr[3], undefi )
The fact that it doesn't do what I expect is reason to not make it a top level
function like this. Another reason is that it is a single for loop.
> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1845
> + if (!entry.resource)
> + return
WI.Rect.rectFromClientRect(targetElement.getBoundingClientRect());
I still feel like this will fall over in some way but I'm not exactly sure how
to test. Maybe some kind of table update that happens while the popover is
showing. (An incomplete request becoming complete?)
> LayoutTests/http/tests/inspector/dom/didFireEvent.html:18
> + let videoNode = null;
Style: I normally drop the assignment on these "values to be populated"
let videoNode;
More information about the webkit-reviews
mailing list