Title: [237409] trunk/Source/WebCore
- Revision
- 237409
- Author
- [email protected]
- Date
- 2018-10-25 01:34:54 -0700 (Thu, 25 Oct 2018)
Log Message
InspectorCanvas is not getting cleared properly for OffscreenCanvas
https://bugs.webkit.org/show_bug.cgi?id=190894
<rdar://problem/45498435>
Patch by Joseph Pecoraro <[email protected]> on 2018-10-25
Reviewed by Simon Fraser.
Covered by existing tests not crashing with guard malloc.
InspectorCanvasAgents tracks all CanvasRenderingContexts and needs to
remove its reference when the containing CanvasBase goes away. It does
this by registering as a notification observer, but if it can't map
from the CanvasBase back to the rendering context we were failing to
remove our reference. Enforce CanvasBase classes to notify observers
of destruction while they still have their CanvasRenderingContext.
* html/CanvasBase.cpp:
(WebCore::CanvasBase::~CanvasBase):
(WebCore::CanvasBase::notifyObserversCanvasDestroyed):
* html/CanvasBase.h:
Assert that subclasses notify observers of the canvas being destroyed,
since they will need to do this before m_context is cleared.
* html/CustomPaintCanvas.cpp:
(WebCore::CustomPaintCanvas::~CustomPaintCanvas):
* html/OffscreenCanvas.cpp:
(WebCore::OffscreenCanvas::~OffscreenCanvas):
Follow the new expected pattern of notifying observers before clearing
the context. HTMLCanvasElement already followed this pattern.
* inspector/agents/InspectorCanvasAgent.cpp:
(WebCore::InspectorCanvasAgent::canvasDestroyed):
Add an assertion that would catch this earlier.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (237408 => 237409)
--- trunk/Source/WebCore/ChangeLog 2018-10-25 07:24:59 UTC (rev 237408)
+++ trunk/Source/WebCore/ChangeLog 2018-10-25 08:34:54 UTC (rev 237409)
@@ -1,3 +1,38 @@
+2018-10-25 Joseph Pecoraro <[email protected]>
+
+ InspectorCanvas is not getting cleared properly for OffscreenCanvas
+ https://bugs.webkit.org/show_bug.cgi?id=190894
+ <rdar://problem/45498435>
+
+ Reviewed by Simon Fraser.
+
+ Covered by existing tests not crashing with guard malloc.
+
+ InspectorCanvasAgents tracks all CanvasRenderingContexts and needs to
+ remove its reference when the containing CanvasBase goes away. It does
+ this by registering as a notification observer, but if it can't map
+ from the CanvasBase back to the rendering context we were failing to
+ remove our reference. Enforce CanvasBase classes to notify observers
+ of destruction while they still have their CanvasRenderingContext.
+
+ * html/CanvasBase.cpp:
+ (WebCore::CanvasBase::~CanvasBase):
+ (WebCore::CanvasBase::notifyObserversCanvasDestroyed):
+ * html/CanvasBase.h:
+ Assert that subclasses notify observers of the canvas being destroyed,
+ since they will need to do this before m_context is cleared.
+
+ * html/CustomPaintCanvas.cpp:
+ (WebCore::CustomPaintCanvas::~CustomPaintCanvas):
+ * html/OffscreenCanvas.cpp:
+ (WebCore::OffscreenCanvas::~OffscreenCanvas):
+ Follow the new expected pattern of notifying observers before clearing
+ the context. HTMLCanvasElement already followed this pattern.
+
+ * inspector/agents/InspectorCanvasAgent.cpp:
+ (WebCore::InspectorCanvasAgent::canvasDestroyed):
+ Add an assertion that would catch this earlier.
+
2018-10-24 Alexey Proskuryakov <[email protected]>
Clean up some obsolete macOS version guards
Modified: trunk/Source/WebCore/html/CanvasBase.cpp (237408 => 237409)
--- trunk/Source/WebCore/html/CanvasBase.cpp 2018-10-25 07:24:59 UTC (rev 237408)
+++ trunk/Source/WebCore/html/CanvasBase.cpp 2018-10-25 08:34:54 UTC (rev 237409)
@@ -42,7 +42,8 @@
CanvasBase::~CanvasBase()
{
ASSERT(!m_context); // Should have been set to null by base class.
- notifyObserversCanvasDestroyed();
+ ASSERT(m_didNotifyObserversCanvasDestroyed);
+ ASSERT(m_observers.isEmpty());
}
CanvasRenderingContext* CanvasBase::renderingContext() const
@@ -80,10 +81,16 @@
void CanvasBase::notifyObserversCanvasDestroyed()
{
+ ASSERT(!m_didNotifyObserversCanvasDestroyed);
+
for (auto& observer : m_observers)
observer->canvasDestroyed(*this);
m_observers.clear();
+
+#ifndef NDEBUG
+ m_didNotifyObserversCanvasDestroyed = true;
+#endif
}
HashSet<Element*> CanvasBase::cssCanvasClients() const
Modified: trunk/Source/WebCore/html/CanvasBase.h (237408 => 237409)
--- trunk/Source/WebCore/html/CanvasBase.h 2018-10-25 07:24:59 UTC (rev 237408)
+++ trunk/Source/WebCore/html/CanvasBase.h 2018-10-25 08:34:54 UTC (rev 237409)
@@ -82,7 +82,7 @@
void removeObserver(CanvasObserver&);
void notifyObserversCanvasChanged(const FloatRect&);
void notifyObserversCanvasResized();
- void notifyObserversCanvasDestroyed();
+ void notifyObserversCanvasDestroyed(); // Must be called in destruction before clearing m_context.
HashSet<Element*> cssCanvasClients() const;
@@ -102,6 +102,9 @@
private:
bool m_originClean { true };
+#ifndef NDEBUG
+ bool m_didNotifyObserversCanvasDestroyed { false };
+#endif
ScriptExecutionContext* m_scriptExecutionContext;
HashSet<CanvasObserver*> m_observers;
};
Modified: trunk/Source/WebCore/html/CustomPaintCanvas.cpp (237408 => 237409)
--- trunk/Source/WebCore/html/CustomPaintCanvas.cpp 2018-10-25 07:24:59 UTC (rev 237408)
+++ trunk/Source/WebCore/html/CustomPaintCanvas.cpp 2018-10-25 08:34:54 UTC (rev 237409)
@@ -46,6 +46,8 @@
CustomPaintCanvas::~CustomPaintCanvas()
{
+ notifyObserversCanvasDestroyed();
+
m_context = nullptr; // Ensure this goes away before the ImageBuffer.
}
Modified: trunk/Source/WebCore/html/OffscreenCanvas.cpp (237408 => 237409)
--- trunk/Source/WebCore/html/OffscreenCanvas.cpp 2018-10-25 07:24:59 UTC (rev 237408)
+++ trunk/Source/WebCore/html/OffscreenCanvas.cpp 2018-10-25 08:34:54 UTC (rev 237409)
@@ -45,6 +45,8 @@
OffscreenCanvas::~OffscreenCanvas()
{
+ notifyObserversCanvasDestroyed();
+
m_context = nullptr;
}
Modified: trunk/Source/WebCore/inspector/agents/InspectorCanvasAgent.cpp (237408 => 237409)
--- trunk/Source/WebCore/inspector/agents/InspectorCanvasAgent.cpp 2018-10-25 07:24:59 UTC (rev 237408)
+++ trunk/Source/WebCore/inspector/agents/InspectorCanvasAgent.cpp 2018-10-25 08:34:54 UTC (rev 237409)
@@ -475,6 +475,7 @@
void InspectorCanvasAgent::canvasDestroyed(CanvasBase& canvasBase)
{
auto* context = canvasBase.renderingContext();
+ ASSERT(context);
if (!context)
return;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
