Modified: trunk/Source/WebKit2/ChangeLog (203720 => 203721)
--- trunk/Source/WebKit2/ChangeLog 2016-07-26 16:23:09 UTC (rev 203720)
+++ trunk/Source/WebKit2/ChangeLog 2016-07-26 16:25:26 UTC (rev 203721)
@@ -1,3 +1,38 @@
+2016-07-26 Carlos Garcia Campos <[email protected]>
+
+ [Threaded Compositor] ASSERTION FAILED: isMainThread() when ThreadedCompositor is destroyed since r203718
+ https://bugs.webkit.org/show_bug.cgi?id=160197
+
+ Reviewed by Žan Doberšek.
+
+ ThreadedCompositor can be destroyed from a secondary thread, for example, when a task takes a reference and the
+ main threads derefs it, when the task finishes in the secondary thread the lambda ends up deleting the threaded
+ compositor. This is ok for the Threaded compositor but not for the CompositingRunLoop class. this was not a
+ problem before r203718 because the CompositingRunLoop object was created and destroyed in the same thread
+ always, but now it's part of the ThreadedCompositor class. This patch uses std:unique_ptr again to explicitly
+ create the CompositingRunLoop in the ThreadedCompositor constructor and delete in the invalidate() method to
+ make sure it happens in the main thread in both cases.
+
+ * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
+ (WebKit::WorkQueuePool::invalidate):
+ (WebKit::WorkQueuePool::getOrCreateWorkQueueForContext):
+ * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+ (WebKit::ThreadedCompositor::ThreadedCompositor):
+ (WebKit::ThreadedCompositor::invalidate):
+ (WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing):
+ (WebKit::ThreadedCompositor::setDeviceScaleFactor):
+ (WebKit::ThreadedCompositor::setDrawsBackground):
+ (WebKit::ThreadedCompositor::didChangeViewportSize):
+ (WebKit::ThreadedCompositor::didChangeViewportAttribute):
+ (WebKit::ThreadedCompositor::didChangeContentsSize):
+ (WebKit::ThreadedCompositor::scrollTo):
+ (WebKit::ThreadedCompositor::scrollBy):
+ (WebKit::ThreadedCompositor::updateViewport):
+ (WebKit::ThreadedCompositor::scheduleDisplayImmediately):
+ (WebKit::ThreadedCompositor::forceRepaint):
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
+
2016-07-26 Youenn Fablet <[email protected]>
Remove ClientCredentialPolicy cross-origin option from ResourceLoaderOptions
Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp (203720 => 203721)
--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp 2016-07-26 16:23:09 UTC (rev 203720)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp 2016-07-26 16:25:26 UTC (rev 203721)
@@ -60,7 +60,7 @@
void invalidate(void* context)
{
auto workQueue = m_workQueueMap.take(context);
- RELEASE_ASSERT(workQueue);
+ ASSERT(workQueue);
if (m_workQueueMap.isEmpty()) {
m_sharedWorkQueue = nullptr;
m_threadCount = 0;
@@ -85,7 +85,7 @@
// FIXME: This is OK for now, and it works for a single-thread limit. But for configurations where more (but not unlimited)
// threads could be used, one option would be to use a HashSet here and disperse the contexts across the available threads.
if (m_threadCount >= m_threadCountLimit) {
- RELEASE_ASSERT(m_sharedWorkQueue);
+ ASSERT(m_sharedWorkQueue);
addResult.iterator->value = m_sharedWorkQueue;
} else {
addResult.iterator->value = WorkQueue::create("org.webkit.ThreadedCompositorWorkQueue");
Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h (203720 => 203721)
--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h 2016-07-26 16:23:09 UTC (rev 203720)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h 2016-07-26 16:25:26 UTC (rev 203721)
@@ -29,6 +29,7 @@
#if USE(COORDINATED_GRAPHICS_THREADED)
#include <wtf/Condition.h>
+#include <wtf/FastMalloc.h>
#include <wtf/Function.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/Noncopyable.h>
@@ -38,6 +39,7 @@
class CompositingRunLoop {
WTF_MAKE_NONCOPYABLE(CompositingRunLoop);
+ WTF_MAKE_FAST_ALLOCATED;
public:
enum UpdateTiming {
Immediate,
Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (203720 => 203721)
--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2016-07-26 16:23:09 UTC (rev 203720)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2016-07-26 16:25:26 UTC (rev 203721)
@@ -48,9 +48,9 @@
ThreadedCompositor::ThreadedCompositor(Client* client)
: m_client(client)
- , m_compositingRunLoop([this] { renderLayerTree(); })
+ , m_compositingRunLoop(std::make_unique<CompositingRunLoop>([this] { renderLayerTree(); }))
{
- m_compositingRunLoop.performTaskSync([this, protectedThis = makeRef(*this)] {
+ m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this)] {
m_scene = adoptRef(new CoordinatedGraphicsScene(this));
m_viewportController = std::make_unique<SimpleViewportController>(this);
});
@@ -64,19 +64,20 @@
void ThreadedCompositor::invalidate()
{
m_scene->detach();
- m_compositingRunLoop.stopUpdateTimer();
- m_compositingRunLoop.performTaskSync([this, protectedThis = makeRef(*this)] {
+ m_compositingRunLoop->stopUpdateTimer();
+ m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this)] {
m_context = nullptr;
m_scene = nullptr;
m_viewportController = nullptr;
});
+ m_compositingRunLoop = nullptr;
m_client = nullptr;
}
void ThreadedCompositor::setNativeSurfaceHandleForCompositing(uint64_t handle)
{
- m_compositingRunLoop.stopUpdateTimer();
- m_compositingRunLoop.performTaskSync([this, protectedThis = makeRef(*this), handle] {
+ m_compositingRunLoop->stopUpdateTimer();
+ m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this), handle] {
m_scene->setActive(!!handle);
// A new native handle can't be set without destroying the previous one first if any.
@@ -89,7 +90,7 @@
void ThreadedCompositor::setDeviceScaleFactor(float scale)
{
- m_compositingRunLoop.performTask([this, protectedThis = makeRef(*this), scale] {
+ m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), scale] {
m_deviceScaleFactor = scale;
scheduleDisplayImmediately();
});
@@ -97,7 +98,7 @@
void ThreadedCompositor::setDrawsBackground(bool drawsBackground)
{
- m_compositingRunLoop.performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), drawsBackground] {
+ m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), drawsBackground] {
m_drawsBackground = drawsBackground;
scheduleDisplayImmediately();
});
@@ -105,7 +106,7 @@
void ThreadedCompositor::didChangeViewportSize(const IntSize& size)
{
- m_compositingRunLoop.performTaskSync([this, protectedThis = makeRef(*this), size] {
+ m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this), size] {
m_viewportController->didChangeViewportSize(size);
});
}
@@ -112,7 +113,7 @@
void ThreadedCompositor::didChangeViewportAttribute(const ViewportAttributes& attr)
{
- m_compositingRunLoop.performTask([this, protectedThis = makeRef(*this), attr] {
+ m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), attr] {
m_viewportController->didChangeViewportAttribute(attr);
});
}
@@ -119,7 +120,7 @@
void ThreadedCompositor::didChangeContentsSize(const IntSize& size)
{
- m_compositingRunLoop.performTask([this, protectedThis = makeRef(*this), size] {
+ m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), size] {
m_viewportController->didChangeContentsSize(size);
});
}
@@ -126,7 +127,7 @@
void ThreadedCompositor::scrollTo(const IntPoint& position)
{
- m_compositingRunLoop.performTask([this, protectedThis = makeRef(*this), position] {
+ m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), position] {
m_viewportController->scrollTo(position);
});
}
@@ -133,7 +134,7 @@
void ThreadedCompositor::scrollBy(const IntSize& delta)
{
- m_compositingRunLoop.performTask([this, protectedThis = makeRef(*this), delta] {
+ m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), delta] {
m_viewportController->scrollBy(delta);
});
}
@@ -158,12 +159,12 @@
void ThreadedCompositor::updateViewport()
{
- m_compositingRunLoop.startUpdateTimer(CompositingRunLoop::WaitUntilNextFrame);
+ m_compositingRunLoop->startUpdateTimer(CompositingRunLoop::WaitUntilNextFrame);
}
void ThreadedCompositor::scheduleDisplayImmediately()
{
- m_compositingRunLoop.startUpdateTimer(CompositingRunLoop::Immediate);
+ m_compositingRunLoop->startUpdateTimer(CompositingRunLoop::Immediate);
}
bool ThreadedCompositor::tryEnsureGLContext()
@@ -198,7 +199,7 @@
void ThreadedCompositor::forceRepaint()
{
- m_compositingRunLoop.performTaskSync([this, protectedThis = makeRef(*this)] {
+ m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this)] {
renderLayerTree();
});
}
