Skip to content

Commit

Permalink
Cherry-pick 259548.787@safari-7615-branch (88ed382). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=257234

    Ensure CanvasBase remains alive while in use
    https://bugs.webkit.org/show_bug.cgi?id=257234
    rdar://109540621

    Reviewed by Chris Dumez.

    A HTMLCanvasElement could be destroyed when it calls CanvasBase::setImageBuffer
    because that call could trigger a GC. We've seen crashes originating from
    HTMLCanvasElement::setImageBufferAndMarkDirty, but this patch adds protection
    around other setImageBuffer call sites, as well.

    * Source/WebCore/html/CanvasBase.h:
    (WebCore::CanvasBase::ref):
    (WebCore::CanvasBase::deref):
    * Source/WebCore/html/HTMLCanvasElement.h:
    * Source/WebCore/html/OffscreenCanvas.cpp:
    (WebCore::OffscreenCanvas::create):
    (WebCore::OffscreenCanvas::setPlaceholderCanvas):
    (WebCore::OffscreenCanvas::pushBufferToPlaceholder):
    * Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
    (WebCore::CanvasRenderingContext2DBase::drawImage):
    * Source/WebCore/rendering/style/StyleCanvasImage.cpp:
    (WebCore::StyleCanvasImage::image const):

    Canonical link: https://commits.webkit.org/259548.787@safari-7615-branch
  • Loading branch information
sysrqb authored and mcatanzaro committed Jul 28, 2023
1 parent 5898307 commit 83d9025
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 8 deletions.
2 changes: 2 additions & 0 deletions Source/WebCore/html/CanvasBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ class CanvasBase {

virtual void refCanvasBase() = 0;
virtual void derefCanvasBase() = 0;
void ref() { refCanvasBase(); }
void deref() { derefCanvasBase(); }

virtual bool isHTMLCanvasElement() const { return false; }
virtual bool isOffscreenCanvas() const { return false; }
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/html/HTMLCanvasElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ class HTMLCanvasElement final : public HTMLElement, public CanvasBase, public Ac
#endif
WEBCORE_EXPORT void setAvoidIOSurfaceSizeCheckInWebProcessForTesting();

using HTMLElement::ref;
using HTMLElement::deref;

private:
HTMLCanvasElement(const QualifiedName&, Document&);

Expand Down
17 changes: 10 additions & 7 deletions Source/WebCore/html/OffscreenCanvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ Ref<OffscreenCanvas> OffscreenCanvas::create(ScriptExecutionContext& scriptExecu
clone->setOriginTainted();

callOnMainThread([detachedCanvas = WTFMove(detachedCanvas), placeholderData = Ref { *clone->m_placeholderData }] () mutable {
placeholderData->canvas = detachedCanvas->takePlaceholderCanvas();
if (placeholderData->canvas) {
auto& placeholderContext = downcast<PlaceholderRenderingContext>(*placeholderData->canvas->renderingContext());
if (RefPtr canvas = detachedCanvas->takePlaceholderCanvas().get()) {
placeholderData->canvas = canvas;
auto& placeholderContext = downcast<PlaceholderRenderingContext>(*canvas->renderingContext());
auto& imageBufferPipe = placeholderContext.imageBufferPipe();
if (imageBufferPipe)
placeholderData->bufferPipeSource = imageBufferPipe->source();
Expand Down Expand Up @@ -471,6 +471,7 @@ void OffscreenCanvas::setPlaceholderCanvas(HTMLCanvasElement& canvas)
{
ASSERT(!m_context);
ASSERT(isMainThread());
Ref protectedCanvas { canvas };
m_placeholderData->canvas = canvas;
auto& placeholderContext = downcast<PlaceholderRenderingContext>(*canvas.renderingContext());
auto& imageBufferPipe = placeholderContext.imageBufferPipe();
Expand All @@ -482,10 +483,12 @@ void OffscreenCanvas::pushBufferToPlaceholder()
{
callOnMainThread([placeholderData = Ref { *m_placeholderData }] () mutable {
Locker locker { placeholderData->bufferLock };
if (placeholderData->canvas && placeholderData->canvas->document().page() && placeholderData->pendingCommitBuffer) {
GraphicsClient& client = placeholderData->canvas->document().page()->chrome();
auto imageBuffer = client.sinkIntoImageBuffer(WTFMove(placeholderData->pendingCommitBuffer));
placeholderData->canvas->setImageBufferAndMarkDirty(WTFMove(imageBuffer));
if (RefPtr canvas = placeholderData->canvas.get()) {
if (canvas->document().page() && placeholderData->pendingCommitBuffer) {
GraphicsClient& client = canvas->document().page()->chrome();
auto imageBuffer = client.sinkIntoImageBuffer(WTFMove(placeholderData->pendingCommitBuffer));
canvas->setImageBufferAndMarkDirty(WTFMove(imageBuffer));
}
}
placeholderData->pendingCommitBuffer = nullptr;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1677,6 +1677,7 @@ ExceptionOr<void> CanvasRenderingContext2DBase::drawImage(CanvasBase& sourceCanv
if (!state().hasInvertibleTransform)
return { };

Ref protectedCanvas { sourceCanvas };
// FIXME: Do this through platform-independent GraphicsContext API.
ImageBuffer* buffer = sourceCanvas.buffer();
if (!buffer)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/style/StyleCanvasImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ RefPtr<Image> StyleCanvasImage::image(const RenderElement* renderer, const Float
return &Image::nullImage();

ASSERT(clients().contains(const_cast<RenderElement*>(renderer)));
auto* element = this->element(renderer->document());
RefPtr element = this->element(renderer->document());
if (!element || !element->buffer())
return nullptr;
return element->copiedImage();
Expand Down

0 comments on commit 83d9025

Please sign in to comment.