Changeset 252792 in webkit

Timestamp:

Nov 22, 2019, 12:29:25 PM (6 years ago)

Author:

Devin Rousso

Message:

Web Inspector: Canvas: adjust InspectorCanvasAgent::recordCanvasAction to actually check that the CanvasRenderingContext still exists
​https://bugs.webkit.org/show_bug.cgi?id=204395
<rdar://problem/57347504>

Reviewed by Ryosuke Niwa.

The microtask we create to handle marking the end of a frame for a detached canvas doesn't
actually check that the CanvasRenderingContext still exists, such as if it gets GCd during
a recording.

Instead of creating a microtask per-canvas, create a microtask per-InspectorCanvasAgent
and save a list of canvasId that are actively recording, which can then be iterated to
handle marking the end of a frame for each once the microtask fires. If the canvasId can't
be re-found, then the related CanvasRenderingContext must have been removed.

• inspector/agents/InspectorCanvasAgent.cpp:

(WebCore::InspectorCanvasAgent::recordCanvasAction):
(WebCore::InspectorCanvasAgent::didFinishRecordingCanvasFrame):
(WebCore::InspectorCanvasAgent::reset):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• inspector/agents/InspectorCanvasAgent.cpp (modified) (6 diffs)
• inspector/agents/InspectorCanvasAgent.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-11-22  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Canvas: adjust `InspectorCanvasAgent::recordCanvasAction` to actually check that the `CanvasRenderingContext` still exists
+        https://bugs.webkit.org/show_bug.cgi?id=204395
+        <rdar://problem/57347504>
+
+        Reviewed by Ryosuke Niwa.
+
+        The microtask we create to handle marking the end of a frame for a detached canvas doesn't
+        actually check that the `CanvasRenderingContext` still exists, such as if it gets GCd during
+        a recording.
+
+        Instead of creating a microtask per-canvas, create a microtask per-`InspectorCanvasAgent`
+        and save a list of `canvasId` that are actively recording, which can then be iterated to
+        handle marking the end of a frame for each once the microtask fires. If the `canvasId` can't
+        be re-found, then the related `CanvasRenderingContext` must have been removed.
+
+        * inspector/agents/InspectorCanvasAgent.cpp:
+        (WebCore::InspectorCanvasAgent::recordCanvasAction):
+        (WebCore::InspectorCanvasAgent::didFinishRecordingCanvasFrame):
+        (WebCore::InspectorCanvasAgent::reset):
+
 2019-11-22  Sihui Liu  <sihui_liu@apple.com>
 

trunk/Source/WebCore/inspector/agents/InspectorCanvasAgent.cpp

@@ -27 +27 @@
 #include "InspectorCanvasAgent.h"
 
-#include "ActiveDOMCallbackMicrotask.h"
 #include "CanvasRenderingContext.h"
 #include "CanvasRenderingContext2D.h"
@@ -41 +40 @@
 #include "InstrumentingAgents.h"
 #include "JSExecState.h"
-#include "Microtasks.h"
 #include "ScriptState.h"
 #include "StringAdaptors.h"
@@ -451 +449 @@
         return;
 
-    // Only enqueue a microtask for the first action of each frame. Any subsequent actions will be
-    // covered by the initial microtask until the next frame.
-    if (!inspectorCanvas->currentFrameHasData()) {
+    // Only enqueue one microtask for all actively recording canvases.
+    if (m_recordingCanvasIdentifiers.isEmpty()) {
         if (auto* scriptExecutionContext = inspectorCanvas->scriptExecutionContext()) {
-            auto& eventLoop = scriptExecutionContext->eventLoop();
-            auto microtask = makeUnique<ActiveDOMCallbackMicrotask>(eventLoop.microtaskQueue(), *scriptExecutionContext, [&, protectedInspectorCanvas = inspectorCanvas.copyRef()] {
-                if (auto* canvasElement = protectedInspectorCanvas->canvasElement()) {
-                    if (canvasElement->isDescendantOf(canvasElement->document()))
-                        return;
+            scriptExecutionContext->eventLoop().queueMicrotask([weakThis = makeWeakPtr(*this)] {
+                if (!weakThis)
+                    return;
+
+                auto& canvasAgent = *weakThis;
+
+                for (auto& identifier : canvasAgent.m_recordingCanvasIdentifiers) {
+                    auto inspectorCanvas = canvasAgent.m_identifierToInspectorCanvas.get(identifier);
+                    if (!inspectorCanvas)
+                        continue;
+
+                    auto* canvasRenderingContext = inspectorCanvas->canvasContext();
+                    ASSERT(canvasRenderingContext);
+                    // FIXME: <https://webkit.org/b/201651> Web Inspector: Canvas: support canvas recordings for WebGPUDevice
+
+                    if (canvasRenderingContext->callTracingActive())
+                        canvasAgent.didFinishRecordingCanvasFrame(*canvasRenderingContext);
                 }
 
-                if (canvasRenderingContext.callTracingActive())
-                    didFinishRecordingCanvasFrame(canvasRenderingContext);
+                canvasAgent.m_recordingCanvasIdentifiers.clear();
             });
-            eventLoop.queueMicrotaskCallback(WTFMove(microtask));
         }
     }
+
+    m_recordingCanvasIdentifiers.add(inspectorCanvas->identifier());
 
     inspectorCanvas->recordAction(name, WTFMove(parameters));
@@ -517 +526 @@
             m_frontendDispatcher->recordingFinished(inspectorCanvas->identifier(), nullptr);
             inspectorCanvas->resetRecordingData();
+            ASSERT(!m_recordingCanvasIdentifiers.contains(inspectorCanvas->identifier()));
         }
         return;
@@ -532 +542 @@
 
     m_frontendDispatcher->recordingFinished(inspectorCanvas->identifier(), inspectorCanvas->releaseObjectForRecording());
+
+    m_recordingCanvasIdentifiers.remove(inspectorCanvas->identifier());
 }
 
@@ -743 +755 @@
     if (m_programDestroyedTimer.isActive())
         m_programDestroyedTimer.stop();
+
+    m_recordingCanvasIdentifiers.clear();
 }
 

trunk/Source/WebCore/inspector/agents/InspectorCanvasAgent.h

@@ -35 +35 @@
 #include <initializer_list>
 #include <wtf/Forward.h>
+#include <wtf/WeakPtr.h>
 
 namespace Inspector {
@@ -57 +58 @@
 typedef String ErrorString;
 
-class InspectorCanvasAgent final : public InspectorAgentBase, public Inspector::CanvasBackendDispatcherHandler, public CanvasObserver {
+class InspectorCanvasAgent final : public InspectorAgentBase, public Inspector::CanvasBackendDispatcherHandler, public CanvasObserver, public CanMakeWeakPtr<InspectorCanvasAgent> {
     WTF_MAKE_NONCOPYABLE(InspectorCanvasAgent);
     WTF_MAKE_FAST_ALLOCATED;
@@ -158 +159 @@
     Timer m_programDestroyedTimer;
 
+    HashSet<String> m_recordingCanvasIdentifiers;
+
     Optional<size_t> m_recordingAutoCaptureFrameCount;
 };