call to setNeedsLayout during RenderVideo::paintReplaced
authorfischman@chromium.org <fischman@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2012 21:07:02 +0000 (21:07 +0000)
committerfischman@chromium.org <fischman@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2012 21:07:02 +0000 (21:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100265

Reviewed by Eric Carlson.

Removed unnecessary call and added new defensive guards to catch erroneous setNeedsLayout() calls
during paints earlier (so the offending calls are in the emitted stacktrace).

No new tests - new defensive checks are triggered by existing tests.

* page/FrameView.cpp:
(WebCore::FrameView::paintContents): forbid setNeedsLayout() during painting
* rendering/RenderObject.cpp:
(WebCore):
(WebCore::RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope):
(WebCore::RenderObject::SetLayoutNeededForbiddenScope::~SetLayoutNeededForbiddenScope):
* rendering/RenderObject.h:
(RenderObject):
(SetLayoutNeededForbiddenScope): added helper class for forbidding setNeedsLayout() in a scope.
* rendering/RenderVideo.cpp:
(WebCore::RenderVideo::paintReplaced): drop the offending & unnecessary call to updatePlayer().

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

Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderObject.h
Source/WebCore/rendering/RenderVideo.cpp

index 2169a1f..d643f11 100644 (file)
@@ -1,3 +1,27 @@
+2012-10-24  Ami Fischman  <fischman@chromium.org>
+
+        call to setNeedsLayout during RenderVideo::paintReplaced
+        https://bugs.webkit.org/show_bug.cgi?id=100265
+
+        Reviewed by Eric Carlson.
+
+        Removed unnecessary call and added new defensive guards to catch erroneous setNeedsLayout() calls
+        during paints earlier (so the offending calls are in the emitted stacktrace).
+
+        No new tests - new defensive checks are triggered by existing tests.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::paintContents): forbid setNeedsLayout() during painting
+        * rendering/RenderObject.cpp:
+        (WebCore):
+        (WebCore::RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope):
+        (WebCore::RenderObject::SetLayoutNeededForbiddenScope::~SetLayoutNeededForbiddenScope):
+        * rendering/RenderObject.h:
+        (RenderObject):
+        (SetLayoutNeededForbiddenScope): added helper class for forbidding setNeedsLayout() in a scope.
+        * rendering/RenderVideo.cpp:
+        (WebCore::RenderVideo::paintReplaced): drop the offending & unnecessary call to updatePlayer().
+
 2012-10-24  Adam Barth  <abarth@webkit.org>
 
         [V8] ActiveDOMObjectEpilogueVisitor is unnecessary and can be deleted
index 739028d..98180c6 100644 (file)
@@ -3242,6 +3242,10 @@ void FrameView::paintContents(GraphicsContext* p, const IntRect& rect)
     RenderObject* eltRenderer = m_nodeToDraw ? m_nodeToDraw->renderer() : 0;
     RenderLayer* rootLayer = root->layer();
 
+#ifndef NDEBUG
+    RenderObject::SetLayoutNeededForbiddenScope forbidSetNeedsLayout(rootLayer->renderer());
+#endif
+
     rootLayer->paint(p, rect, m_paintBehavior, eltRenderer);
 
     if (rootLayer->containsDirtyOverlayScrollbars())
index 318a149..e649d13 100644 (file)
@@ -91,6 +91,18 @@ using namespace HTMLNames;
 
 #ifndef NDEBUG
 static void* baseOfRenderObjectBeingDeleted;
+
+RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope(RenderObject* renderObject)
+    : m_renderObject(renderObject)
+    , m_preexistingForbidden(m_renderObject->isSetNeedsLayoutForbidden())
+{
+    m_renderObject->setNeedsLayoutIsForbidden(true);
+}
+
+RenderObject::SetLayoutNeededForbiddenScope::~SetLayoutNeededForbiddenScope()
+{
+    m_renderObject->setNeedsLayoutIsForbidden(m_preexistingForbidden);
+}
 #endif
 
 struct SameSizeAsRenderObject {
index 1d5e767..9768ea2 100644 (file)
@@ -248,6 +248,16 @@ public:
     bool hasAXObject() const { return m_hasAXObject; }
     bool isSetNeedsLayoutForbidden() const { return m_setNeedsLayoutForbidden; }
     void setNeedsLayoutIsForbidden(bool flag) { m_setNeedsLayoutForbidden = flag; }
+
+    // Helper class forbidding calls to setNeedsLayout() during its lifetime.
+    class SetLayoutNeededForbiddenScope {
+    public:
+        explicit SetLayoutNeededForbiddenScope(RenderObject*);
+        ~SetLayoutNeededForbiddenScope();
+    private:
+        RenderObject* m_renderObject;
+        bool m_preexistingForbidden;
+    };
 #endif
 
     // Obtains the nearest enclosing block (including this block) that contributes a first-line style to our inline
index 02c313b..545b143 100644 (file)
@@ -195,13 +195,10 @@ void RenderVideo::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
     if (Frame* frame = this->frame())
         page = frame->page();
 
-    if (!displayingPoster) {
-        if (!mediaPlayer) {
-            if (page && paintInfo.phase == PaintPhaseForeground)
-                page->addRelevantUnpaintedObject(this, visualOverflowRect());
-            return;
-        }
-        updatePlayer();
+    if (!displayingPoster && !mediaPlayer) {
+        if (page && paintInfo.phase == PaintPhaseForeground)
+            page->addRelevantUnpaintedObject(this, visualOverflowRect());
+        return;
     }
 
     LayoutRect rect = videoBox();