[StressGC] ASSERTION FAILED: m_wrapper under WebCore::WebGLRenderingContextBase:...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2020 18:30:08 +0000 (18:30 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2020 18:30:08 +0000 (18:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209660
<rdar://problem/60541733>

Reviewed by Darin Adler.

Make HTMLCanvasElement an ActiveDOMObject since WebGLRenderingContextBase needs to dispatch events
asynchronously on its canvas element. Update WebGLRenderingContextBase to use the HTML event loop
to dispatch those events asynchronously instead of using suspendible timers.

No new tests, already covered by webgl/max-active-contexts-webglcontextlost-prevent-default.html.

* dom/TaskSource.h:
* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::HTMLCanvasElement):
(WebCore::HTMLCanvasElement::create):
(WebCore::HTMLCanvasElement::activeDOMObjectName const):
* html/HTMLCanvasElement.h:
* html/HTMLCanvasElement.idl:
* html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::WebGLRenderingContextBase):
(WebCore::WebGLRenderingContextBase::loseContextImpl):
(WebCore::WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent):
(WebCore::WebGLRenderingContextBase::dispatchContextChangedNotification):
(WebCore::WebGLRenderingContextBase::dispatchContextLostEvent): Deleted.
(WebCore::WebGLRenderingContextBase::dispatchContextChangedEvent): Deleted.
* html/canvas/WebGLRenderingContextBase.h:

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

Source/WebCore/ChangeLog
Source/WebCore/dom/TaskSource.h
Source/WebCore/html/HTMLCanvasElement.cpp
Source/WebCore/html/HTMLCanvasElement.h
Source/WebCore/html/HTMLCanvasElement.idl
Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
Source/WebCore/html/canvas/WebGLRenderingContextBase.h

index f14749b..b6bdb37 100644 (file)
@@ -1,3 +1,33 @@
+2020-03-27  Chris Dumez  <cdumez@apple.com>
+
+        [StressGC] ASSERTION FAILED: m_wrapper under WebCore::WebGLRenderingContextBase::dispatchContextLostEvent
+        https://bugs.webkit.org/show_bug.cgi?id=209660
+        <rdar://problem/60541733>
+
+        Reviewed by Darin Adler.
+
+        Make HTMLCanvasElement an ActiveDOMObject since WebGLRenderingContextBase needs to dispatch events
+        asynchronously on its canvas element. Update WebGLRenderingContextBase to use the HTML event loop
+        to dispatch those events asynchronously instead of using suspendible timers.
+
+        No new tests, already covered by webgl/max-active-contexts-webglcontextlost-prevent-default.html.
+
+        * dom/TaskSource.h:
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::HTMLCanvasElement):
+        (WebCore::HTMLCanvasElement::create):
+        (WebCore::HTMLCanvasElement::activeDOMObjectName const):
+        * html/HTMLCanvasElement.h:
+        * html/HTMLCanvasElement.idl:
+        * html/canvas/WebGLRenderingContextBase.cpp:
+        (WebCore::WebGLRenderingContextBase::WebGLRenderingContextBase):
+        (WebCore::WebGLRenderingContextBase::loseContextImpl):
+        (WebCore::WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent):
+        (WebCore::WebGLRenderingContextBase::dispatchContextChangedNotification):
+        (WebCore::WebGLRenderingContextBase::dispatchContextLostEvent): Deleted.
+        (WebCore::WebGLRenderingContextBase::dispatchContextChangedEvent): Deleted.
+        * html/canvas/WebGLRenderingContextBase.h:
+
 2020-03-27  Simon Fraser  <simon.fraser@apple.com>
 
         Use Optional<> for a lazily-computed bounds rectangle
index d9c80ad..8548eee 100644 (file)
@@ -38,6 +38,7 @@ enum class TaskSource : uint8_t {
     Networking,
     PostedMessageQueue,
     UserInteraction,
+    WebGL,
 
     // Internal to WebCore
     InternalAsyncTask, // Safe to re-order or delay.
index 3545616..e6a2268 100644 (file)
@@ -113,18 +113,23 @@ static size_t maxActivePixelMemoryForTesting = 0;
 HTMLCanvasElement::HTMLCanvasElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
     , CanvasBase(IntSize(defaultWidth, defaultHeight))
+    , ActiveDOMObject(document)
 {
     ASSERT(hasTagName(canvasTag));
 }
 
 Ref<HTMLCanvasElement> HTMLCanvasElement::create(Document& document)
 {
-    return adoptRef(*new HTMLCanvasElement(canvasTag, document));
+    auto canvas = adoptRef(*new HTMLCanvasElement(canvasTag, document));
+    canvas->suspendIfNeeded();
+    return canvas;
 }
 
 Ref<HTMLCanvasElement> HTMLCanvasElement::create(const QualifiedName& tagName, Document& document)
 {
-    return adoptRef(*new HTMLCanvasElement(tagName, document));
+    auto canvas = adoptRef(*new HTMLCanvasElement(tagName, document));
+    canvas->suspendIfNeeded();
+    return canvas;
 }
 
 HTMLCanvasElement::~HTMLCanvasElement()
@@ -945,4 +950,9 @@ void HTMLCanvasElement::clearCopiedImage()
     m_didClearImageBuffer = false;
 }
 
+const char* HTMLCanvasElement::activeDOMObjectName() const
+{
+    return "HTMLCanvasElement";
+}
+
 }
index 42134a1..2e62891 100644 (file)
@@ -27,6 +27,7 @@
 
 #pragma once
 
+#include "ActiveDOMObject.h"
 #include "CanvasBase.h"
 #include "FloatRect.h"
 #include "HTMLElement.h"
@@ -57,7 +58,7 @@ namespace DisplayList {
 using AsTextFlags = unsigned;
 }
 
-class HTMLCanvasElement final : public HTMLElement, public CanvasBase {
+class HTMLCanvasElement final : public HTMLElement, public CanvasBase, public ActiveDOMObject {
     WTF_MAKE_ISO_ALLOCATED(HTMLCanvasElement);
 public:
     static Ref<HTMLCanvasElement> create(Document&);
@@ -132,6 +133,7 @@ private:
     HTMLCanvasElement(const QualifiedName&, Document&);
 
     bool isHTMLCanvasElement() const final { return true; }
+    const char* activeDOMObjectName() const final;
 
     void parseAttribute(const QualifiedName&, const AtomString&) final;
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
index 69f0a71..718a9ea 100644 (file)
@@ -38,6 +38,7 @@ typedef (
     CanvasRenderingContext2D) RenderingContext;
 
 [
+    ActiveDOMObject,
     JSGenerateToNativeObject,
     ReportExtraMemoryCost,
     ReportExternalMemoryCost,
index 2f8adfa..bac9249 100644 (file)
@@ -611,16 +611,12 @@ std::unique_ptr<WebGLRenderingContextBase> WebGLRenderingContextBase::create(Can
 
 WebGLRenderingContextBase::WebGLRenderingContextBase(CanvasBase& canvas, WebGLContextAttributes attributes)
     : GPUBasedCanvasRenderingContext(canvas)
-    , m_dispatchContextLostEventTimer(canvas.scriptExecutionContext(), *this, &WebGLRenderingContextBase::dispatchContextLostEvent)
-    , m_dispatchContextChangedEventTimer(canvas.scriptExecutionContext(), *this, &WebGLRenderingContextBase::dispatchContextChangedEvent)
     , m_restoreTimer(canvas.scriptExecutionContext(), *this, &WebGLRenderingContextBase::maybeRestoreContext)
     , m_attributes(attributes)
     , m_numGLErrorsToConsoleAllowed(maxGLErrorsAllowedToConsole)
     , m_isPendingPolicyResolution(true)
     , m_checkForContextLossHandlingTimer(*this, &WebGLRenderingContextBase::checkForContextLossHandling)
 {
-    m_dispatchContextLostEventTimer.suspendIfNeeded();
-    m_dispatchContextChangedEventTimer.suspendIfNeeded();
     m_restoreTimer.suspendIfNeeded();
 
     registerWithWebGLStateTracker();
@@ -630,16 +626,12 @@ WebGLRenderingContextBase::WebGLRenderingContextBase(CanvasBase& canvas, WebGLCo
 WebGLRenderingContextBase::WebGLRenderingContextBase(CanvasBase& canvas, Ref<GraphicsContextGLOpenGL>&& context, WebGLContextAttributes attributes)
     : GPUBasedCanvasRenderingContext(canvas)
     , m_context(WTFMove(context))
-    , m_dispatchContextLostEventTimer(canvas.scriptExecutionContext(), *this, &WebGLRenderingContextBase::dispatchContextLostEvent)
-    , m_dispatchContextChangedEventTimer(canvas.scriptExecutionContext(), *this, &WebGLRenderingContextBase::dispatchContextChangedEvent)
     , m_restoreTimer(canvas.scriptExecutionContext(), *this, &WebGLRenderingContextBase::maybeRestoreContext)
     , m_generatedImageCache(4)
     , m_attributes(attributes)
     , m_numGLErrorsToConsoleAllowed(maxGLErrorsAllowedToConsole)
     , m_checkForContextLossHandlingTimer(*this, &WebGLRenderingContextBase::checkForContextLossHandling)
 {
-    m_dispatchContextLostEventTimer.suspendIfNeeded();
-    m_dispatchContextChangedEventTimer.suspendIfNeeded();
     m_restoreTimer.suspendIfNeeded();
 
     m_contextGroup = WebGLContextGroup::create();
@@ -5195,7 +5187,7 @@ void WebGLRenderingContextBase::loseContextImpl(WebGLRenderingContextBase::LostC
 
     // Always defer the dispatch of the context lost event, to implement
     // the spec behavior of queueing a task.
-    m_dispatchContextLostEventTimer.startOneShot(0_s);
+    scheduleTaskToDispatchContextLostEvent();
 }
 
 void WebGLRenderingContextBase::forceRestoreContext()
@@ -6318,18 +6310,20 @@ void WebGLRenderingContextBase::restoreStatesAfterVertexAttrib0Simulation()
 }
 #endif
 
-void WebGLRenderingContextBase::dispatchContextLostEvent()
+void WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent()
 {
-    RELEASE_ASSERT(!m_isSuspended);
     auto* canvas = htmlCanvas();
     if (!canvas)
         return;
 
-    Ref<WebGLContextEvent> event = WebGLContextEvent::create(eventNames().webglcontextlostEvent, Event::CanBubble::No, Event::IsCancelable::Yes, emptyString());
-    canvas->dispatchEvent(event);
-    m_restoreAllowed = event->defaultPrevented();
-    if (m_contextLostMode == RealLostContext && m_restoreAllowed)
-        m_restoreTimer.startOneShot(0_s);
+    // It is safe to capture |this| because we keep the canvas element alive and it owns |this|.
+    queueTaskKeepingObjectAlive(*canvas, TaskSource::WebGL, [this, canvas] {
+        auto event = WebGLContextEvent::create(eventNames().webglcontextlostEvent, Event::CanBubble::No, Event::IsCancelable::Yes, emptyString());
+        canvas->dispatchEvent(event);
+        m_restoreAllowed = event->defaultPrevented();
+        if (m_contextLostMode == RealLostContext && m_restoreAllowed)
+            m_restoreTimer.startOneShot(0_s);
+    });
 }
 
 void WebGLRenderingContextBase::maybeRestoreContext()
@@ -6726,18 +6720,11 @@ void WebGLRenderingContextBase::recycleContext()
 
 void WebGLRenderingContextBase::dispatchContextChangedNotification()
 {
-    if (!m_dispatchContextChangedEventTimer.isActive())
-        m_dispatchContextChangedEventTimer.startOneShot(0_s);
-}
-
-void WebGLRenderingContextBase::dispatchContextChangedEvent()
-{
-    RELEASE_ASSERT(!m_isSuspended);
     auto* canvas = htmlCanvas();
     if (!canvas)
         return;
 
-    canvas->dispatchEvent(WebGLContextEvent::create(eventNames().webglcontextchangedEvent, Event::CanBubble::No, Event::IsCancelable::Yes, emptyString()));
+    queueTaskToDispatchEvent(*canvas, TaskSource::WebGL, WebGLContextEvent::create(eventNames().webglcontextchangedEvent, Event::CanBubble::No, Event::IsCancelable::Yes, emptyString()));
 }
 
 
index 7af25da..dd21d72 100644 (file)
@@ -462,13 +462,6 @@ protected:
     RefPtr<GraphicsContextGLOpenGL> m_context;
     RefPtr<WebGLContextGroup> m_contextGroup;
 
-    // Dispatches a context lost event once it is determined that one is needed.
-    // This is used both for synthetic and real context losses. For real ones, it's
-    // likely that there's no JavaScript on the stack, but that might be dependent
-    // on how exactly the platform discovers that the context was lost. For better
-    // portability we always defer the dispatch of the event.
-    SuspendableTimer m_dispatchContextLostEventTimer;
-    SuspendableTimer m_dispatchContextChangedEventTimer;
     bool m_restoreAllowed { false };
     SuspendableTimer m_restoreTimer;
 
@@ -864,8 +857,7 @@ protected:
     template <typename T> unsigned getMaxIndex(const RefPtr<JSC::ArrayBuffer> elementArrayBuffer, GCGLintptr uoffset, GCGLsizei n);
 
 private:
-    void dispatchContextLostEvent();
-    void dispatchContextChangedEvent();
+    void scheduleTaskToDispatchContextLostEvent();
     // Helper for restoration after context lost.
     void maybeRestoreContext();