[webkit-reviews] review granted: [Bug 228224] [GPU Process] Start tracking resource uses for NativeImages and Fonts : [Attachment 434309] Patch

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 228224: [GPU Process] Start tracking resource uses for NativeImages and
Fonts
https://bugs.webkit.org/show_bug.cgi?id=228224

Attachment 434309: Patch

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




--- Comment #5 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 434309
  --> https://bugs.webkit.org/attachment.cgi?id=434309
Patch

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

> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp:105
> +    if (delegate) {

Maybe an early return will look clearer

if (!delegate)
    return std::nullopt;

> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp:108
> +	   if (changeFlags.contains(GraphicsContextState::StrokePatternChange))
> +	      
delegate->recordResourceUse(strokePatternRenderingResourceIdentifier);

Can't this be written like this:

if (strokePatternRenderingResourceIdentifier)
    delegate->recordResourceUse(strokePatternRenderingResourceIdentifier);

> Source/WebKit/WebProcess/GPU/graphics/RemoteResourceCacheProxy.cpp:81
>  void RemoteResourceCacheProxy::cacheNativeImage(NativeImage& image)

Maybe we need to choose new names for these functions since they do more than
just caching the resources:

cacheNativeImageAndUse()
ensureNativeImageIsCachedAndUse()
recordNativeImageUse()

Or something similar. This may be also done in a separate patch.