Source/WebCore:
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Feb 2019 21:57:48 +0000 (21:57 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Feb 2019 21:57:48 +0000 (21:57 +0000)
Remove setDefersLoading infrastructure from WebKit2
https://bugs.webkit.org/show_bug.cgi?id=194506

Patch by Alex Christensen <achristensen@webkit.org> on 2019-02-12
Reviewed by Brady Eidson.

setDefersLoading is inherently racy from WebCore to the NetworkProcess,
it adds unwanted complexity to the initialization and use of network objects,
and it has led to many unrecoverable hang bugs over the years.
We needed to force it into WebKit2 to transition some existing clients who relied on it,
but we have recently finished transitioning those clients to other solutions, mostly
completion handlers.

* inspector/PageScriptDebugServer.cpp:
(WebCore::PageScriptDebugServer::setJavaScriptPaused):

LayoutTests:
BitmapRenderer should handle existing ImageBuffers
https://bugs.webkit.org/show_bug.cgi?id=194555
<rdar://problem/47857150>

Reviewed by Tim Horton.

Test that creates a canvas, triggers an ImageBuffer to be created, then
creates the bitmaprenderer context.

* fast/canvas/bitmaprenderer-created-after-toBlob-expected.txt: Added.
* fast/canvas/bitmaprenderer-created-after-toBlob.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241319 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/canvas/bitmaprenderer-created-after-toBlob-expected.txt [new file with mode: 0644]
LayoutTests/fast/canvas/bitmaprenderer-created-after-toBlob.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLCanvasElement.cpp

index 5c9e534..b7f2bc3 100644 (file)
@@ -1,3 +1,17 @@
+2019-02-12  Dean Jackson  <dino@apple.com>
+
+        BitmapRenderer should handle existing ImageBuffers
+        https://bugs.webkit.org/show_bug.cgi?id=194555
+        <rdar://problem/47857150>
+
+        Reviewed by Tim Horton.
+
+        Test that creates a canvas, triggers an ImageBuffer to be created, then
+        creates the bitmaprenderer context.
+
+        * fast/canvas/bitmaprenderer-created-after-toBlob-expected.txt: Added.
+        * fast/canvas/bitmaprenderer-created-after-toBlob.html: Added.
+
 2019-02-12  Alex Christensen  <achristensen@webkit.org>
 
         Remove setDefersLoading infrastructure from WebKit2
diff --git a/LayoutTests/fast/canvas/bitmaprenderer-created-after-toBlob-expected.txt b/LayoutTests/fast/canvas/bitmaprenderer-created-after-toBlob-expected.txt
new file mode 100644 (file)
index 0000000..3b3302d
--- /dev/null
@@ -0,0 +1 @@
+Should not crash.
diff --git a/LayoutTests/fast/canvas/bitmaprenderer-created-after-toBlob.html b/LayoutTests/fast/canvas/bitmaprenderer-created-after-toBlob.html
new file mode 100644 (file)
index 0000000..a139f50
--- /dev/null
@@ -0,0 +1,22 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function callback() {
+    if (window.GCController)
+        return GCController.collect();
+
+    // Force garbage collection
+    for (let i=0; i < 100; i++)
+        var a = new Uint8Array(1024*1024);
+}
+
+function run() {
+   var c = document.createElement("canvas");
+   c.toBlob(callback);
+   var ctx = c.getContext("bitmaprenderer");
+}
+
+window.addEventListener("load", run, false);
+</script>
+Should not crash.
index 8907311..b3f96ee 100644 (file)
         * WebCore.xcodeproj/project.pbxproj:
         * bindings/js/WebCoreBuiltinNames.h:
 
-2019-02-06  Dean Jackson  <dino@apple.com>
+2019-02-12  Dean Jackson  <dino@apple.com>
 
-        Fix potential build error in GPUDevice
-        https://bugs.webkit.org/show_bug.cgi?id=194359
+        BitmapRenderer should handle existing ImageBuffers
+        https://bugs.webkit.org/show_bug.cgi?id=194555
+        <rdar://problem/47857150>
 
-        Reviewed by Joseph Pecoraro.
+        Reviewed by Tim Horton.
 
-        Add an UNUSED_PARAM for non-macOS platforms.
+        Our logic in ImageBitmapRenderingContext assumed that
+        it had always created the ImageBuffer being used. However, it's
+        valid to call something like toBlob() or toDataURL() before creating
+        a context, so we need to handle the case where an ImageBuffer
+        already exists.
 
-        * platform/graphics/gpu/cocoa/GPUDeviceMetal.mm:
-        (WebCore::GPUDevice::create):
+        Test: fast/canvas/bitmaprenderer-created-after-toBlob.html
+
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::createImageBuffer const): Move some logic into setImageBuffer.
+        (WebCore::HTMLCanvasElement::setImageBuffer const): Make sure to clear the state saver.
 
 2019-02-06  Daniel Bates  <dabates@apple.com>
 
index 7a70b24..2680210 100644 (file)
@@ -1002,22 +1002,6 @@ void HTMLCanvasElement::createImageBuffer() const
 
     auto hostWindow = (document().view() && document().view()->root()) ? document().view()->root()->hostWindow() : nullptr;
     setImageBuffer(ImageBuffer::create(size(), renderingMode, 1, ColorSpaceSRGB, hostWindow));
-    if (!m_imageBuffer)
-        return;
-    m_imageBuffer->context().setShadowsIgnoreTransforms(true);
-    m_imageBuffer->context().setImageInterpolationQuality(defaultInterpolationQuality);
-    m_imageBuffer->context().setStrokeThickness(1);
-    m_contextStateSaver = std::make_unique<GraphicsContextStateSaver>(m_imageBuffer->context());
-
-    JSC::JSLockHolder lock(HTMLElement::scriptExecutionContext()->vm());
-    HTMLElement::scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(memoryCost());
-
-#if USE(IOSURFACE_CANVAS_BACKING_STORE) || ENABLE(ACCELERATED_2D_CANVAS)
-    if (m_context && m_context->is2d()) {
-        // Recalculate compositing requirements if acceleration state changed.
-        const_cast<HTMLCanvasElement*>(this)->invalidateStyleAndLayerComposition();
-    }
-#endif
 }
 
 void HTMLCanvasElement::setImageBuffer(std::unique_ptr<ImageBuffer>&& buffer) const
@@ -1027,6 +1011,7 @@ void HTMLCanvasElement::setImageBuffer(std::unique_ptr<ImageBuffer>&& buffer) co
 
     {
         auto locker = holdLock(m_imageBufferAssignmentLock);
+        m_contextStateSaver = nullptr;
         m_imageBuffer = WTFMove(buffer);
     }
 
@@ -1038,6 +1023,23 @@ void HTMLCanvasElement::setImageBuffer(std::unique_ptr<ImageBuffer>&& buffer) co
 
     if (m_context && m_imageBuffer && previousMemoryCost != currentMemoryCost)
         InspectorInstrumentation::didChangeCanvasMemory(*m_context);
+
+    if (!m_imageBuffer)
+        return;
+    m_imageBuffer->context().setShadowsIgnoreTransforms(true);
+    m_imageBuffer->context().setImageInterpolationQuality(defaultInterpolationQuality);
+    m_imageBuffer->context().setStrokeThickness(1);
+    m_contextStateSaver = std::make_unique<GraphicsContextStateSaver>(m_imageBuffer->context());
+
+    JSC::JSLockHolder lock(HTMLElement::scriptExecutionContext()->vm());
+    HTMLElement::scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(memoryCost());
+
+#if USE(IOSURFACE_CANVAS_BACKING_STORE) || ENABLE(ACCELERATED_2D_CANVAS)
+    if (m_context && m_context->is2d()) {
+        // Recalculate compositing requirements if acceleration state changed.
+        const_cast<HTMLCanvasElement*>(this)->invalidateStyleAndLayerComposition();
+    }
+#endif
 }
 
 void HTMLCanvasElement::setImageBufferAndMarkDirty(std::unique_ptr<ImageBuffer>&& buffer)