[Webkit-unassigned] [Bug 239350] Web Inspector: [Meta] Implement Timelines Film Strip

[bugzilla-daemon at webkit.org]

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

--- Comment #18 from Patrick Angle <pangle at apple.com> ---
Comment on attachment 459078
  --> https://bugs.webkit.org/attachment.cgi?id=459078
Patch v1.4

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

> Source/JavaScriptCore/ChangeLog:3
> -2022-05-06  Yusuke Suzuki  <ysuzuki at apple.com>
> +2022-05-09  Yusuke Suzuki  <ysuzuki at apple.com>
>  
> -        [JSC] Add more information about MarkedBlock assertion
> +        Web Inspector: [Meta] Implement Timelines Film Strip

Something has gone wrong here (and in the other changelog files as well). WebInspectorUI changeling is the only one that appears to be correct right now.

> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:122
> +        // if (WI.ScreenshotsInstrument.supported())

I'm pretty sure we still need this check to avoid allowing this timeline on OSes that don't support it.

> Source/WebInspectorUI/UserInterface/Views/ContentView.js:106
> +

Style: Unnecessary extra newline here. The first new newline is good though.

> Source/WebInspectorUI/UserInterface/Views/TimelineTabContentView.js:-270
> -

Style: We should keep this newline to separate this from the default case below (even though these will all fall through anyways).

> LayoutTests/inspector/timeline/timeline-event-ScreenShots-expected.txt:8
> +!! TIMEOUT: took longer than 10000ms

err... Does this file need updated, or are these the actual results you are seeing from the test? The test should execute successfully, which means no timeout, just test results. In this case I would expect the expectations to end with something like:
```
PASS: Screenshot record height should be non-zero.
```

-- 
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/20220509/859dc56b/attachment.htm>