- Revision
- 112178
- Author
- fsam...@chromium.org
- Date
- 2012-03-26 17:09:12 -0700 (Mon, 26 Mar 2012)
Log Message
[Chromium] Using WebViewPlugins with --force-compositing-mode causes an ASSERT to fail
https://bugs.webkit.org/show_bug.cgi?id=81954
Reviewed by James Robinson.
A static variable s_inPaintContents is set when painting, and it ensures
that we don't delete GraphicsLayers or create GraphicsLayers while painting.
However, because this variable is static, it does not permit the existence
of multiple WebViews in different stages (one laying out and one painting).
This manifests itself if one attempts to use the --force-compositing-mode
in Chromium and attempts to navigate to a page with a missing or old plugin
or a browser plugin (which uses a WebViewPlugin as a placeholder until it's
done loading).
The solution to simplify debugging is to make this flag per-Page.
We can access Page from RenderLayerBacking which is a GraphicsLayerClient.
We add a new method GraphicsLayerClient::verifyNotPainting with a default
(do nothing) implementation and override it in RenderLayerBacking to
test the flag set in Page.
* page/Page.cpp:
(WebCore::Page::Page):
* page/Page.h:
(Page):
(WebCore::Page::setIsPainting):
(WebCore::Page::isPainting):
* platform/graphics/GraphicsLayer.cpp:
(WebCore::GraphicsLayer::GraphicsLayer):
(WebCore::GraphicsLayer::~GraphicsLayer):
(WebCore::GraphicsLayer::paintGraphicsLayerContents):
* platform/graphics/GraphicsLayerClient.h:
(GraphicsLayerClient):
(WebCore::GraphicsLayerClient::verifyNotPainting):
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::paintContents):
(WebCore):
(WebCore::RenderLayerBacking::verifyNotPainting):
* rendering/RenderLayerBacking.h:
(RenderLayerBacking):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (112177 => 112178)
--- trunk/Source/WebCore/ChangeLog 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/ChangeLog 2012-03-27 00:09:12 UTC (rev 112178)
@@ -1,3 +1,47 @@
+2012-03-26 Fady Samuel <fsam...@chromium.org>
+
+ [Chromium] Using WebViewPlugins with --force-compositing-mode causes an ASSERT to fail
+ https://bugs.webkit.org/show_bug.cgi?id=81954
+
+ Reviewed by James Robinson.
+
+ A static variable s_inPaintContents is set when painting, and it ensures
+ that we don't delete GraphicsLayers or create GraphicsLayers while painting.
+
+ However, because this variable is static, it does not permit the existence
+ of multiple WebViews in different stages (one laying out and one painting).
+
+ This manifests itself if one attempts to use the --force-compositing-mode
+ in Chromium and attempts to navigate to a page with a missing or old plugin
+ or a browser plugin (which uses a WebViewPlugin as a placeholder until it's
+ done loading).
+
+ The solution to simplify debugging is to make this flag per-Page.
+ We can access Page from RenderLayerBacking which is a GraphicsLayerClient.
+ We add a new method GraphicsLayerClient::verifyNotPainting with a default
+ (do nothing) implementation and override it in RenderLayerBacking to
+ test the flag set in Page.
+
+ * page/Page.cpp:
+ (WebCore::Page::Page):
+ * page/Page.h:
+ (Page):
+ (WebCore::Page::setIsPainting):
+ (WebCore::Page::isPainting):
+ * platform/graphics/GraphicsLayer.cpp:
+ (WebCore::GraphicsLayer::GraphicsLayer):
+ (WebCore::GraphicsLayer::~GraphicsLayer):
+ (WebCore::GraphicsLayer::paintGraphicsLayerContents):
+ * platform/graphics/GraphicsLayerClient.h:
+ (GraphicsLayerClient):
+ (WebCore::GraphicsLayerClient::verifyNotPainting):
+ * rendering/RenderLayerBacking.cpp:
+ (WebCore::RenderLayerBacking::paintContents):
+ (WebCore):
+ (WebCore::RenderLayerBacking::verifyNotPainting):
+ * rendering/RenderLayerBacking.h:
+ (RenderLayerBacking):
+
2012-03-23 Ryosuke Niwa <rn...@webkit.org>
cssText should use shorthand notations
Modified: trunk/Source/WebCore/page/Page.cpp (112177 => 112178)
--- trunk/Source/WebCore/page/Page.cpp 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/page/Page.cpp 2012-03-27 00:09:12 UTC (rev 112178)
@@ -160,6 +160,9 @@
#endif
, m_displayID(0)
, m_isCountingRelevantRepaintedObjects(false)
+#ifndef NDEBUG
+ , m_isPainting(false)
+#endif
{
if (!allPages) {
allPages = new HashSet<Page*>;
Modified: trunk/Source/WebCore/page/Page.h (112177 => 112178)
--- trunk/Source/WebCore/page/Page.h 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/page/Page.h 2012-03-27 00:09:12 UTC (rev 112178)
@@ -331,7 +331,10 @@
void suspendActiveDOMObjectsAndAnimations();
void resumeActiveDOMObjectsAndAnimations();
-
+#ifndef NDEBUG
+ void setIsPainting(bool painting) { m_isPainting = painting; }
+ bool isPainting() const { return m_isPainting; }
+#endif
private:
void initGroup();
@@ -431,6 +434,9 @@
Region m_relevantPaintedRegion;
Region m_relevantUnpaintedRegion;
bool m_isCountingRelevantRepaintedObjects;
+#ifndef NDEBUG
+ bool m_isPainting;
+#endif
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp (112177 => 112178)
--- trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp 2012-03-27 00:09:12 UTC (rev 112178)
@@ -63,10 +63,6 @@
m_values.append(value);
}
-#ifndef NDEBUG
-static bool s_inPaintContents = false;
-#endif
-
GraphicsLayer::GraphicsLayer(GraphicsLayerClient* client)
: m_client(client)
, m_anchorPoint(0.5f, 0.5f, 0)
@@ -91,12 +87,18 @@
, m_replicatedLayer(0)
, m_repaintCount(0)
{
- ASSERT(!s_inPaintContents);
+#ifndef NDEBUG
+ if (m_client)
+ m_client->verifyNotPainting();
+#endif
}
GraphicsLayer::~GraphicsLayer()
{
- ASSERT(!s_inPaintContents);
+#ifndef NDEBUG
+ if (m_client)
+ m_client->verifyNotPainting();
+#endif
removeAllChildren();
removeFromParent();
}
@@ -288,9 +290,6 @@
void GraphicsLayer::paintGraphicsLayerContents(GraphicsContext& context, const IntRect& clip)
{
-#ifndef NDEBUG
- s_inPaintContents = true;
-#endif
if (m_client) {
LayoutSize offset = offsetFromRenderer();
context.translate(-offset);
@@ -300,9 +299,6 @@
m_client->paintContents(this, context, m_paintingPhase, pixelSnappedIntRect(clipRect));
}
-#ifndef NDEBUG
- s_inPaintContents = false;
-#endif
}
String GraphicsLayer::animationNameForTransition(AnimatedPropertyID property)
Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h (112177 => 112178)
--- trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h 2012-03-27 00:09:12 UTC (rev 112178)
@@ -74,6 +74,15 @@
virtual bool showDebugBorders(const GraphicsLayer*) const = 0;
virtual bool showRepaintCounter(const GraphicsLayer*) const = 0;
+
+#ifndef NDEBUG
+ // RenderLayerBacking overrides this to verify that it is not
+ // currently painting contents. An ASSERT fails, if it is.
+ // This is executed in GraphicsLayer construction and destruction
+ // to verify that we don't create or destroy GraphicsLayers
+ // while painting.
+ virtual void verifyNotPainting() { }
+#endif
};
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (112177 => 112178)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp 2012-03-27 00:09:12 UTC (rev 112178)
@@ -1160,6 +1160,10 @@
// Up-call from compositing layer drawing callback.
void RenderLayerBacking::paintContents(const GraphicsLayer* graphicsLayer, GraphicsContext& context, GraphicsLayerPaintingPhase paintingPhase, const IntRect& clip)
{
+#ifndef NDEBUG
+ if (Page* page = renderer()->frame()->page())
+ page->setIsPainting(true);
+#endif
if (graphicsLayer == m_graphicsLayer.get() || graphicsLayer == m_foregroundLayer.get() || graphicsLayer == m_maskLayer.get()) {
InspectorInstrumentationCookie cookie = InspectorInstrumentation::willPaint(m_owningLayer->renderer()->frame(), &context, clip);
@@ -1185,6 +1189,10 @@
m_owningLayer->paintResizer(&context, IntPoint(), transformedClip);
context.restore();
}
+#ifndef NDEBUG
+ if (Page* page = renderer()->frame()->page())
+ page->setIsPainting(false);
+#endif
}
float RenderLayerBacking::pageScaleFactor() const
@@ -1212,6 +1220,13 @@
return compositor() ? compositor()->compositorShowRepaintCounter() : false;
}
+#ifndef NDEBUG
+void RenderLayerBacking::verifyNotPainting()
+{
+ ASSERT(!renderer()->frame()->page() || !renderer()->frame()->page()->isPainting());
+}
+#endif
+
bool RenderLayerBacking::startAnimation(double timeOffset, const Animation* anim, const KeyframeList& keyframes)
{
bool hasOpacity = keyframes.containsProperty(CSSPropertyOpacity);
Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.h (112177 => 112178)
--- trunk/Source/WebCore/rendering/RenderLayerBacking.h 2012-03-26 23:59:27 UTC (rev 112177)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.h 2012-03-27 00:09:12 UTC (rev 112178)
@@ -136,6 +136,10 @@
virtual bool showDebugBorders(const GraphicsLayer*) const;
virtual bool showRepaintCounter(const GraphicsLayer*) const;
+#ifndef NDEBUG
+ virtual void verifyNotPainting();
+#endif
+
IntRect contentsBox() const;
// For informative purposes only.
