[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 #5 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 457717
  --> https://bugs.webkit.org/attachment.cgi?id=457717
Patch v1.0

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

Sounds really neat! I couldn't wait to at least read through things and point out super minor stuff.

Are there some screenshots / a video that we can check out?

> Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp:427
> +    if (!m_recordFilmStrip)

Given `didComposite` might want to perform a series of operations, it might be better to not have this early return in the middle. It also probably makes sense to encapsulate this operation in its own method (which might be useful elsewhere). I'm thinking of something like:

```
    if (m_recordFilmStrip) {
        takeFilmStripSnapshot();
    }
```

And the logic for taking a snapshot in `InspectorTimelineAgent::takeFileStripSnapshot`.

> Source/WebInspectorUI/UserInterface/Models/FilmStripInstrument.js:41
> +        // return InspectorBackend.hasDomain("FilmStripProfiler");
> +        return WI.settings.engineeringShowFilmStrip.value;

How about both?

```
return InspectorBackend.hasDomain("FilmStripProfiler") && WI.settings.engineeringShowFilmStrip.value;
```

> Source/WebInspectorUI/UserInterface/Views/FilmStripTimelineOverviewGraph.css:2
> + * Copyright (C) 2019-2020 Apple Inc. All rights reserved.

I suspect these Copyrights should all be 2022 eh?

-- 
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/20220415/d20b8c3e/attachment.htm>