[webkit-reviews] review granted: [Bug 239107] REGRESSION(r282117): RemoteRenderingBackend::willDestroyImageBuffer() can crash if the RemoteRenderingBackend has already been destroyed : [Attachment 458631] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 239107: REGRESSION(r282117):
RemoteRenderingBackend::willDestroyImageBuffer() can crash if the
RemoteRenderingBackend has already been destroyed
https://bugs.webkit.org/show_bug.cgi?id=239107

Attachment 458631: Patch

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




--- Comment #17 from Darin Adler <darin at apple.com> ---
Comment on attachment 458631
  --> https://bugs.webkit.org/attachment.cgi?id=458631
Patch

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

I can’t exactly follow whether this changes any whether cases join the pool or
not—I understand the new timing, but hard to follow the logic in the old code
to see which would and would not end up releasing to the pool—but I’m guessing
it’s not changed here.

> Source/WebCore/platform/graphics/cg/IOSurfacePool.cpp:66
> +    return adoptRef(*new IOSurfacePool());

Not changed: Normally we’d omit the () after IOSurfacePool here.

> Source/WebCore/platform/graphics/cg/IOSurfacePool.h:38
> +#include <wtf/RefCounted.h>

I think we’d want to include ThreadSafeRefCounted.h, since that’s what we’re
using, not RefCounted. But apparently it’s already included by something else,
so maybe we don’t need to add an include at all?