[webkit-reviews] review granted: [Bug 206068] REGRESSION (r254291): [ Catalina wk2 Debug ] Flaky ASSERT on fast/images/animated-image-loop-count.html : [Attachment 387509] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 13 14:19:32 PST 2020
Chris Dumez <cdumez at apple.com> has granted Chris Lord <clord at igalia.com>'s
request for review:
Bug 206068: REGRESSION (r254291): [ Catalina wk2 Debug ] Flaky ASSERT on
fast/images/animated-image-loop-count.html
https://bugs.webkit.org/show_bug.cgi?id=206068
Attachment 387509: Patch
https://bugs.webkit.org/attachment.cgi?id=387509&action=review
--- Comment #6 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 387509
--> https://bugs.webkit.org/attachment.cgi?id=387509
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=387509&action=review
I think this would work fine but I have a couple of nits.
> Source/WebCore/platform/graphics/ImageSource.cpp:349
> + decodingQueue().dispatch([protectedThis = makeRef(*this),
protectedDecodingQueue = makeRef(decodingQueue()), protectedFrameRequestQueue =
makeRef(frameRequestQueue()), protectedDecoder = makeRef(*m_decoder), sourceURL
= sourceURL().string().isolatedCopy()] () mutable {
I do not understand why we're capturing the protectedDecodingQueue here, is
does not appear to be needed. WorkQueue::dispatch() already takes care of
ref'ing the WorkQueue object internally.
> Source/WebCore/platform/graphics/ImageSource.cpp:385
> + // Ensure destruction happens on creation thread
Missing period at the end of the comment (per coding style).
> Source/WebCore/platform/graphics/ImageSource.cpp:386
> + protectedThis->m_runLoop.dispatch([protectedThis =
WTFMove(protectedThis), protectedQueue = WTFMove(protectedDecodingQueue),
protectedDecoder = WTFMove(protectedDecoder)] () mutable { });
I see no good reason to dispatch the protectedQueue here, I don't think it
should even have been captured in the first place.
More information about the webkit-reviews
mailing list