[webkit-reviews] review granted: [Bug 174451] Async image decoding for large images should be disabled after the first time a tile is painted : [Attachment 315678] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 174451: Async image decoding for large images should be disabled after the
first time a tile is painted
https://bugs.webkit.org/show_bug.cgi?id=174451

Attachment 315678: Patch

https://bugs.webkit.org/attachment.cgi?id=315678&action=review




--- Comment #14 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 315678
  --> https://bugs.webkit.org/attachment.cgi?id=315678
Patch

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

> Source/WebCore/platform/graphics/ca/TileGrid.cpp:776
> +    return m_tileRepaintCounts.contains(platformCALayer) ?
m_tileRepaintCounts.get(platformCALayer) : 0;

This is two hash lookups. You could do it in one with find().

> Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:1129
> +	   flags = GraphicsLayerPaintFlags::None;

Would be more self-documenting, and correct, as flags &=
~AllowAsyncImageDecoding

> Source/WebCore/rendering/RenderBoxModelObject.cpp:330
>      if (document().isImageDocument())
>	   return DecodingMode::Synchronous;
> +    if (bitmapImage.isLargeImageAsyncDecodingEnabledForTesting())
> +	   return DecodingMode::Asynchronous;

Maybe the testing flag should trump the image document case, so we can test
everything?

> Source/WebCore/rendering/RenderBoxModelObject.cpp:337
> +    if (!isVisibleInViewport())

isVisibleInViewport is not cheap. Maybe add a FIXME noting that.

> Source/WebCore/rendering/RenderLayerBacking.cpp:2584
> +	   flags = GraphicsLayerPaintFlags::None;

Would be more self-documenting, and correct, as flags &=
~AllowAsyncImageDecoding