Changeset 112178 in webkit

Timestamp:

Mar 26, 2012 5:09:12 PM (12 years ago)

Author:

fsamuel@chromium.org

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/Page.cpp (modified) (1 diff)
• page/Page.h (modified) (2 diffs)
• platform/graphics/GraphicsLayer.cpp (modified) (4 diffs)
• platform/graphics/GraphicsLayerClient.h (modified) (1 diff)
• rendering/RenderLayerBacking.cpp (modified) (3 diffs)
• rendering/RenderLayerBacking.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-03-26  Fady Samuel  <fsamuel@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  <rniwa@webkit.org>
 

trunk/Source/WebCore/page/Page.cpp

@@ -161 +161 @@
     , m_displayID(0)
     , m_isCountingRelevantRepaintedObjects(false)
+#ifndef NDEBUG
+    , m_isPainting(false)
+#endif
 {
     if (!allPages) {

trunk/Source/WebCore/page/Page.h

@@ -332 +332 @@
         void suspendActiveDOMObjectsAndAnimations();
         void resumeActiveDOMObjectsAndAnimations();
-
+#ifndef NDEBUG
+        void setIsPainting(bool painting) { m_isPainting = painting; }
+        bool isPainting() const { return m_isPainting; }
+#endif
     private:
         void initGroup();
@@ -432 +435 @@
         Region m_relevantUnpaintedRegion;
         bool m_isCountingRelevantRepaintedObjects;
+#ifndef NDEBUG
+        bool m_isPainting;
+#endif
     };
 

trunk/Source/WebCore/platform/graphics/GraphicsLayer.cpp

@@ -64 +64 @@
 }
 
-#ifndef NDEBUG
-static bool s_inPaintContents = false;
-#endif
-
 GraphicsLayer::GraphicsLayer(GraphicsLayerClient* client)
     : m_client(client)
@@ -92 +88 @@
     , 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();
@@ -289 +291 @@
 void GraphicsLayer::paintGraphicsLayerContents(GraphicsContext& context, const IntRect& clip)
 {
-#ifndef NDEBUG
-    s_inPaintContents = true;
-#endif
     if (m_client) {
         LayoutSize offset = offsetFromRenderer();
@@ -301 +300 @@
         m_client->paintContents(this, context, m_paintingPhase, pixelSnappedIntRect(clipRect));
     }
-#ifndef NDEBUG
-    s_inPaintContents = false;
-#endif
 }
 

trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h

@@ -75 +75 @@
     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
 };
 

trunk/Source/WebCore/rendering/RenderLayerBacking.cpp

@@ -1161 +1161 @@
 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);
@@ -1186 +1190 @@
         context.restore();
     }
+#ifndef NDEBUG
+    if (Page* page = renderer()->frame()->page())
+        page->setIsPainting(false);
+#endif
 }
 
@@ -1212 +1220 @@
     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)

trunk/Source/WebCore/rendering/RenderLayerBacking.h

@@ -137 +137 @@
     virtual bool showRepaintCounter(const GraphicsLayer*) const;
 
+#ifndef NDEBUG
+    virtual void verifyNotPainting();
+#endif
+
     IntRect contentsBox() const;