[Webkit-unassigned] [Bug 244128] Add experimental feature to disable image animations

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 19 14:28:53 PDT 2022


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

Tyler Wilcock <tyler_w at apple.com> changed:

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

--- Comment #4 from Tyler Wilcock <tyler_w at apple.com> ---
Comment on attachment 461738
  --> https://bugs.webkit.org/attachment.cgi?id=461738
Patch

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

> Source/WebCore/Modules/model-element/HTMLModelElement.cpp:509
> +        m_modelPlayer->setAnimationIsPlaying(false, [] (bool success) mutable {

Not 100% sure this will compile, but can you do this:

m_modelPlayer->setAnimationIsPlaying(false, [] (bool) mutable { }

(i.e. not assigning a name to the bool, and therefore not needing UNUSED_PARAM)?

> Source/WebCore/html/HTMLImageElement.cpp:68
> +#include "UserAgentStyleSheets.h"

You added a bunch of new includes in this file -- are they all necessary in this patch? (I know you've split your original patch up intentionally, so maybe artifacts of that?)

> Source/WebCore/html/HTMLImageElement.cpp:425
> +            if (is<BitmapImage>(*image)) {

This can be:

if (auto* bitmapImage = dynamicDowncast<BitmapImage>(image))
    bitmapImage->updateDocumentOnRender(document());

dynamicDowncast will return a nullptr if the object isn't the expected type.

> Source/WebCore/page/Page.cpp:4037
> +            if (is<HTMLImageElement>(element)) {

I think you can use dynamicDowncasts here too.

> Source/WebCore/page/Page.cpp:4057
> +                    if (auto cachedBackgroundImage = renderer->style().backgroundLayers().image()->cachedImage()) {

I believe:

renderer->style().backgroundLayers().image()->cachedImage()

Returns a CachedImage*, so you should use auto* instead of auto.

> Source/WebCore/page/Page.cpp:4058
> +                        if (auto backgroundImage = cachedBackgroundImage->image()) {

I think cachedBackgroundImage->image() also returns a pointer type, so this should also be auto*.

(If any of these return a RefPtr<T> rather than T*, then I believe just `auto` is fine)

> Source/WebCore/page/Page.cpp:4059
> +                            if (backgroundImage && is<BitmapImage>(backgroundImage)) {

I think is<T> returns false for nullptr objects, so the null check is unnecessary.

-- 
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/20220819/f59152b2/attachment-0001.htm>


More information about the webkit-unassigned mailing list