Modified: trunk/LayoutTests/platform/glib/TestExpectations (269045 => 269046)
--- trunk/LayoutTests/platform/glib/TestExpectations 2020-10-27 16:58:03 UTC (rev 269045)
+++ trunk/LayoutTests/platform/glib/TestExpectations 2020-10-27 17:08:50 UTC (rev 269046)
@@ -392,11 +392,6 @@
webkit.org/b/215462 imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.commit.html [ Failure Pass ]
-# Flaky crash on pushBufferToPlaceholder().
-webkit.org/b/217986 imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.line.width.transformed.html [ Crash Pass ]
-webkit.org/b/217986 imported/w3c/web-platform-tests/html/canvas/offscreen/path-objects/2d.path.arc.selfintersect.1.worker.html [ Crash Pass ]
-webkit.org/b/217986 imported/w3c/web-platform-tests/html/canvas/offscreen/path-objects/2d.path.arc.selfintersect.1.html [ Crash Pass ]
-
#////////////////////////////////////////////////////////////////////////////////////////
# End of OffscreenCanvas-related bugs
#////////////////////////////////////////////////////////////////////////////////////////
Modified: trunk/Source/WebCore/ChangeLog (269045 => 269046)
--- trunk/Source/WebCore/ChangeLog 2020-10-27 16:58:03 UTC (rev 269045)
+++ trunk/Source/WebCore/ChangeLog 2020-10-27 17:08:50 UTC (rev 269046)
@@ -1,3 +1,27 @@
+2020-10-27 Chris Lord <[email protected]>
+
+ [GLIB] imported/w3c/web-platform-tests/html/canvas/offscreen/line-styles/2d.line.width.transformed.html is a flaky crash
+ https://bugs.webkit.org/show_bug.cgi?id=217986
+
+ Reviewed by Simon Fraser.
+
+ Fix race condition caused by use of OffscreenCanvas::scriptExecutionContext() on main thread.
+ Instead of passing a reference to the OffscreenCanvas object when dealing with the placeholder
+ canvas on the main thread, encapsulate the necessary data in a separate, ThreadSafeRefCounted
+ object and pass that instead, negating the need to call back to the Worker thread to release
+ the reference.
+
+ Covered by existing tests.
+
+ * html/OffscreenCanvas.cpp:
+ (WebCore::OffscreenCanvas::create):
+ (WebCore::OffscreenCanvas::OffscreenCanvas):
+ (WebCore::OffscreenCanvas::detach):
+ (WebCore::OffscreenCanvas::setPlaceholderCanvas):
+ (WebCore::OffscreenCanvas::pushBufferToPlaceholder):
+ (WebCore::OffscreenCanvas::commitToPlaceholderCanvas):
+ * html/OffscreenCanvas.h:
+
2020-10-27 Antti Koivisto <[email protected]>
[LFC][Integration] Use iterator for next/previousLinePosition
Modified: trunk/Source/WebCore/html/OffscreenCanvas.cpp (269045 => 269046)
--- trunk/Source/WebCore/html/OffscreenCanvas.cpp 2020-10-27 16:58:03 UTC (rev 269045)
+++ trunk/Source/WebCore/html/OffscreenCanvas.cpp 2020-10-27 17:08:50 UTC (rev 269046)
@@ -85,16 +85,14 @@
if (!detachedCanvas->originClean())
clone->setOriginTainted();
- callOnMainThread([detachedCanvas = WTFMove(detachedCanvas), offscreenCanvas = makeRef(clone.get())] () mutable {
- offscreenCanvas->m_placeholderCanvas = detachedCanvas->takePlaceholderCanvas();
- if (offscreenCanvas->m_placeholderCanvas) {
- auto& placeholderContext = downcast<PlaceholderRenderingContext>(*offscreenCanvas->m_placeholderCanvas->renderingContext());
+ callOnMainThread([detachedCanvas = WTFMove(detachedCanvas), placeholderData = makeRef(*clone->m_placeholderData)] () mutable {
+ placeholderData->canvas = detachedCanvas->takePlaceholderCanvas();
+ if (placeholderData->canvas) {
+ auto& placeholderContext = downcast<PlaceholderRenderingContext>(*placeholderData->canvas->renderingContext());
auto& imageBufferPipe = placeholderContext.imageBufferPipe();
if (imageBufferPipe)
- offscreenCanvas->m_bufferPipeSource = imageBufferPipe->source();
+ placeholderData->bufferPipeSource = imageBufferPipe->source();
}
- offscreenCanvas->scriptExecutionContext()->postTask({ ScriptExecutionContext::Task::CleanupTask,
- [releaseOffscreenCanvas = WTFMove(offscreenCanvas)] (ScriptExecutionContext&) { } });
});
return clone;
@@ -110,6 +108,7 @@
OffscreenCanvas::OffscreenCanvas(ScriptExecutionContext& context, unsigned width, unsigned height)
: CanvasBase(IntSize(width, height))
, ContextDestructionObserver(&context)
+ , m_placeholderData(PlaceholderData::create())
{
}
@@ -389,7 +388,7 @@
m_detached = true;
auto detached = makeUnique<DetachedOffscreenCanvas>(takeImageBuffer(), size(), originClean());
- detached->m_placeholderCanvas = std::exchange(m_placeholderCanvas, nullptr);
+ detached->m_placeholderCanvas = std::exchange(m_placeholderData->canvas, nullptr);
return detached;
}
@@ -398,25 +397,20 @@
{
ASSERT(!m_context);
ASSERT(isMainThread());
- m_placeholderCanvas = makeWeakPtr(canvas);
+ m_placeholderData->canvas = makeWeakPtr(canvas);
auto& placeholderContext = downcast<PlaceholderRenderingContext>(*canvas.renderingContext());
auto& imageBufferPipe = placeholderContext.imageBufferPipe();
if (imageBufferPipe)
- m_bufferPipeSource = imageBufferPipe->source();
+ m_placeholderData->bufferPipeSource = imageBufferPipe->source();
}
void OffscreenCanvas::pushBufferToPlaceholder()
{
- callOnMainThread([protectedThis = makeRef(*this), this] () mutable {
- {
- auto locker = holdLock(m_commitLock);
- if (m_placeholderCanvas && m_pendingCommitBuffer)
- m_placeholderCanvas->setImageBufferAndMarkDirty(WTFMove(m_pendingCommitBuffer));
- m_pendingCommitBuffer = nullptr;
- }
-
- scriptExecutionContext()->postTask({ ScriptExecutionContext::Task::CleanupTask,
- [releaseThis = WTFMove(protectedThis)] (ScriptExecutionContext&) { } });
+ callOnMainThread([placeholderData = makeRef(*m_placeholderData)] () mutable {
+ auto locker = holdLock(placeholderData->bufferLock);
+ if (placeholderData->canvas && placeholderData->pendingCommitBuffer)
+ placeholderData->canvas->setImageBufferAndMarkDirty(WTFMove(placeholderData->pendingCommitBuffer));
+ placeholderData->pendingCommitBuffer = nullptr;
});
}
@@ -430,16 +424,16 @@
if (m_context && (m_context->isWebGL() || m_context->isAccelerated()))
m_context->paintRenderingResultsToCanvas();
- if (m_bufferPipeSource) {
+ if (m_placeholderData->bufferPipeSource) {
auto bufferCopy = imageBuffer->copyRectToBuffer(FloatRect(FloatPoint(), imageBuffer->logicalSize()), ColorSpace::SRGB, imageBuffer->context());
if (bufferCopy)
- m_bufferPipeSource->handle(WTFMove(bufferCopy));
+ m_placeholderData->bufferPipeSource->handle(WTFMove(bufferCopy));
}
- auto locker = holdLock(m_commitLock);
- bool shouldPushBuffer = !m_pendingCommitBuffer;
- m_pendingCommitBuffer = imageBuffer->copyRectToBuffer(FloatRect(FloatPoint(), imageBuffer->logicalSize()), ColorSpace::SRGB, imageBuffer->context());
- if (m_pendingCommitBuffer && shouldPushBuffer)
+ auto locker = holdLock(m_placeholderData->bufferLock);
+ bool shouldPushBuffer = !m_placeholderData->pendingCommitBuffer;
+ m_placeholderData->pendingCommitBuffer = imageBuffer->copyRectToBuffer(FloatRect(FloatPoint(), imageBuffer->logicalSize()), ColorSpace::SRGB, imageBuffer->context());
+ if (m_placeholderData->pendingCommitBuffer && shouldPushBuffer)
pushBufferToPlaceholder();
}
Modified: trunk/Source/WebCore/html/OffscreenCanvas.h (269045 => 269046)
--- trunk/Source/WebCore/html/OffscreenCanvas.h 2020-10-27 16:58:03 UTC (rev 269045)
+++ trunk/Source/WebCore/html/OffscreenCanvas.h 2020-10-27 17:08:50 UTC (rev 269046)
@@ -39,6 +39,7 @@
#include "ScriptWrappable.h"
#include <wtf/Forward.h>
#include <wtf/RefCounted.h>
+#include <wtf/ThreadSafeRefCounted.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/WTFString.h>
#if ENABLE(WEBGL)
@@ -187,11 +188,21 @@
mutable RefPtr<Image> m_copiedImage;
bool m_hasScheduledCommit { false };
- WeakPtr<HTMLCanvasElement> m_placeholderCanvas;
- RefPtr<ImageBufferPipe::Source> m_bufferPipeSource;
- mutable Lock m_commitLock;
- std::unique_ptr<ImageBuffer> m_pendingCommitBuffer;
+ class PlaceholderData : public ThreadSafeRefCounted<PlaceholderData> {
+ public:
+ static Ref<PlaceholderData> create()
+ {
+ return adoptRef(*new PlaceholderData);
+ }
+
+ WeakPtr<HTMLCanvasElement> canvas;
+ RefPtr<ImageBufferPipe::Source> bufferPipeSource;
+ std::unique_ptr<ImageBuffer> pendingCommitBuffer;
+ mutable Lock bufferLock;
+ };
+
+ RefPtr<PlaceholderData> m_placeholderData;
};
}
