Changeset 87277 in webkit

Timestamp:

May 25, 2011 12:17:21 AM (13 years ago)

Author:

yuzo@google.com

Message:

2011-05-25 Yuzo Fujishima <​yuzo@google.com>

Reviewed by Kent Tamura.

Fix for Bug 61352 - Refactor RenderView::{enable,disable}LayoutState call sites to use RIIA
​https://bugs.webkit.org/show_bug.cgi?id=61352

No new tests because no behavior changes.

• html/shadow/MediaControlElements.cpp: (WebCore::RenderMediaVolumeSliderContainer::layout):
• page/FrameView.cpp: (WebCore::FrameView::layout):
• rendering/RenderBlock.cpp: (WebCore::RenderBlock::repaintOverhangingFloats): (WebCore::RenderBlock::updateFirstLetter):
• rendering/RenderLayer.cpp: (WebCore::RenderLayer::updateLayerPositions): (WebCore::RenderLayer::updateScrollInfoAfterLayout):
• rendering/RenderListBox.cpp: (WebCore::RenderListBox::layout):
• rendering/RenderListItem.cpp: (WebCore::RenderListItem::updateMarkerLocation):
• rendering/RenderMedia.cpp: (WebCore::RenderMedia::layout):
• rendering/RenderView.h: (WebCore::RenderView::disableLayoutState): (WebCore::RenderView::enableLayoutState): (WebCore::LayoutStateDisabler::LayoutStateDisabler): (WebCore::LayoutStateDisabler::~LayoutStateDisabler):
• rendering/svg/RenderSVGRoot.cpp: (WebCore::RenderSVGRoot::layout):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/shadow/MediaControlElements.cpp (modified) (1 diff)
• page/FrameView.cpp (modified) (1 diff)
• rendering/RenderBlock.cpp (modified) (7 diffs)
• rendering/RenderLayer.cpp (modified) (3 diffs)
• rendering/RenderListBox.cpp (modified) (1 diff)
• rendering/RenderListItem.cpp (modified) (2 diffs)
• rendering/RenderMedia.cpp (modified) (1 diff)
• rendering/RenderView.h (modified) (3 diffs)
• rendering/svg/RenderSVGRoot.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-05-25  Yuzo Fujishima  <yuzo@google.com>
+
+        Reviewed by Kent Tamura.
+
+        Fix for Bug 61352 - Refactor RenderView::{enable,disable}LayoutState call sites to use RIIA
+        https://bugs.webkit.org/show_bug.cgi?id=61352
+
+        No new tests because no behavior changes.
+
+        * html/shadow/MediaControlElements.cpp:
+        (WebCore::RenderMediaVolumeSliderContainer::layout):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layout):
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::repaintOverhangingFloats):
+        (WebCore::RenderBlock::updateFirstLetter):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateLayerPositions):
+        (WebCore::RenderLayer::updateScrollInfoAfterLayout):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::layout):
+        * rendering/RenderListItem.cpp:
+        (WebCore::RenderListItem::updateMarkerLocation):
+        * rendering/RenderMedia.cpp:
+        (WebCore::RenderMedia::layout):
+        * rendering/RenderView.h:
+        (WebCore::RenderView::disableLayoutState):
+        (WebCore::RenderView::enableLayoutState):
+        (WebCore::LayoutStateDisabler::LayoutStateDisabler):
+        (WebCore::LayoutStateDisabler::~LayoutStateDisabler):
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::layout):
+
 2011-05-24  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/Source/WebCore/html/shadow/MediaControlElements.cpp

@@ -168 +168 @@
     RenderBox* buttonBox = toRenderBox(previousSibling());
 
-    if (view())
-        view()->disableLayoutState();
+    LayoutStateDisabler layoutStateDisabler(view());
 
     IntPoint offset = theme()->volumeSliderOffsetFromMuteButton(buttonBox, IntSize(width(), height()));
     setX(offset.x() + buttonBox->offsetLeft());
     setY(offset.y() + buttonBox->offsetTop());
-
-    if (view())
-        view()->enableLayoutState();
 }
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -979 +979 @@
     m_actionScheduler->pause();
 
-    bool disableLayoutState = false;
-    if (subtree) {
-        RenderView* view = root->view();
-        disableLayoutState = view->shouldDisableLayoutStateForSubtree(root);
-        view->pushLayoutState(root);
-        if (disableLayoutState)
-            view->disableLayoutState();
-    }
-        
-    m_inLayout = true;
-    beginDeferredRepaints();
-    root->layout();
-    endDeferredRepaints();
-    m_inLayout = false;
-
-    if (subtree) {
-        RenderView* view = root->view();
-        view->popLayoutState(root);
-        if (disableLayoutState)
-            view->enableLayoutState();
+    {
+        bool disableLayoutState = false;
+        if (subtree) {
+            RenderView* view = root->view();
+            disableLayoutState = view->shouldDisableLayoutStateForSubtree(root);
+            view->pushLayoutState(root);
+        }
+        LayoutStateDisabler layoutStateDisabler(disableLayoutState ? root->view() : 0);
+
+        m_inLayout = true;
+        beginDeferredRepaints();
+        root->layout();
+        endDeferredRepaints();
+        m_inLayout = false;
+
+        if (subtree)
+            root->view()->popLayoutState(root);
     }
     m_layoutRoot = 0;

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -2226 +2226 @@
     // FIXME: Avoid disabling LayoutState. At the very least, don't disable it for floats originating
     // in this block. Better yet would be to push extra state for the containers of other floats.
-    view()->disableLayoutState();
+    LayoutStateDisabler layoutStateDisabler(view());
     FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
     FloatingObjectSetIterator end = floatingObjectSet.end();
@@ -2239 +2239 @@
         }
     }
-    view()->enableLayoutState();
 }
  
@@ -5352 +5351 @@
 
             // Move the first letter into the new renderer.
-            view()->disableLayoutState();
+            LayoutStateDisabler layoutStateDisabler(view());
             while (RenderObject* child = firstLetter->firstChild()) {
                 if (child->isText())
@@ -5359 +5358 @@
                 newFirstLetter->addChild(child, 0);
             }
-            
+
             RenderTextFragment* remainingText = 0;
             RenderObject* nextSibling = firstLetter->nextSibling();
@@ -5378 +5377 @@
             firstLetter = newFirstLetter;
             firstLetterContainer->addChild(firstLetter, nextSibling);
-            view()->enableLayoutState();
         } else
             firstLetter->setStyle(pseudoStyle);
@@ -5398 +5396 @@
     // Our layout state is not valid for the repaints we are going to trigger by
     // adding and removing children of firstLetterContainer.
-    view()->disableLayoutState();
+    LayoutStateDisabler layoutStateDisabler(view());
 
     RenderText* textObj = toRenderText(currChild);
@@ -5461 +5459 @@
         textObj->destroy();
     }
-    view()->enableLayoutState();
 }
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -310 +310 @@
         RenderView* view = renderer()->view();
         ASSERT(view);
-        // FIXME: Optimize using LayoutState and remove the disableLayoutState() call
+        // FIXME: Optimize using LayoutState and remove LayoutStateDisabler instantiation
         // from updateScrollInfoAfterLayout().
         ASSERT(!view->layoutStateEnabled());
@@ -2182 +2182 @@
             // scrollToOffset() may call updateLayerPositions(), which doesn't work
             // with LayoutState.
-            // FIXME: Remove the disableLayoutState/enableLayoutState if the above changes.
-            if (view)
-                view->disableLayoutState();
+            // FIXME: Remove the LayoutStateDisabler instantiation if the above changes.
+            LayoutStateDisabler layoutStateDisabler(view);
             scrollToOffset(newX, newY);
-            if (view)
-                view->enableLayoutState();
         }
     }
@@ -2262 +2259 @@
  
     RenderView* view = renderer()->view();
-    view->disableLayoutState();
-    scrollToOffset(scrollXOffset(), scrollYOffset());
-    view->enableLayoutState();
- 
+    {
+        LayoutStateDisabler layoutStateDisabler(view);
+        scrollToOffset(scrollXOffset(), scrollYOffset());
+    }
+
     if (renderer()->node() && renderer()->document()->hasListenerType(Document::OVERFLOWCHANGED_LISTENER))
         updateOverflowStatus(horizontalOverflow, verticalOverflow);

trunk/Source/WebCore/rendering/RenderListBox.cpp

@@ -151 +151 @@
     RenderBlock::layout();
     if (m_scrollToRevealSelectionAfterLayout) {
-        view()->disableLayoutState();
+        LayoutStateDisabler layoutStateDisabler(view());
         scrollToRevealSelection();
-        view()->enableLayoutState();
     }
 }

trunk/Source/WebCore/rendering/RenderListItem.cpp

@@ -214 +214 @@
             // Removing and adding the marker can trigger repainting in
             // containers other than ourselves, so we need to disable LayoutState.
-            view()->disableLayoutState();
+            LayoutStateDisabler layoutStateDisabler(view());
             updateFirstLetter();
             m_marker->remove();
@@ -222 +222 @@
             if (m_marker->preferredLogicalWidthsDirty())
                 m_marker->computePreferredLogicalWidths();
-            view()->enableLayoutState();
         }
     }

trunk/Source/WebCore/rendering/RenderMedia.cpp

@@ -71 +71 @@
 
     // When calling layout() on a child node, a parent must either push a LayoutStateMaintainter, or 
-    // call view()->disableLayoutState().  Since using a LayoutStateMaintainer is slightly more efficient,
+    // instantiate LayoutStateDisabler. Since using a LayoutStateMaintainer is slightly more efficient,
     // and this method will be called many times per second during playback, use a LayoutStateMaintainer:
     LayoutStateMaintainer statePusher(view(), this, IntSize(x(), y()), hasTransform() || hasReflection() || style()->isFlippedBlocksWritingMode());

trunk/Source/WebCore/rendering/RenderView.h

@@ -126 +126 @@
     LayoutState* layoutState() const { return m_layoutState; }
 
-    // Suspends the LayoutState optimization. Used under transforms that cannot be represented by
-    // LayoutState (common in SVG) and when manipulating the render tree during layout in ways
-    // that can trigger repaint of a non-child (e.g. when a list item moves its list marker around).
-    // Note that even when disabled, LayoutState is still used to store layoutDelta.
-    void disableLayoutState() { m_layoutStateDisableCount++; }
-    void enableLayoutState() { ASSERT(m_layoutStateDisableCount > 0); m_layoutStateDisableCount--; }
-
     virtual void updateHitTestResult(HitTestResult&, const IntPoint&);
 
@@ -196 +189 @@
     }
 
+    // Suspends the LayoutState optimization. Used under transforms that cannot be represented by
+    // LayoutState (common in SVG) and when manipulating the render tree during layout in ways
+    // that can trigger repaint of a non-child (e.g. when a list item moves its list marker around).
+    // Note that even when disabled, LayoutState is still used to store layoutDelta.
+    // These functions may only be accessed by LayoutStateMaintainer or LayoutStateDisabler.
+    void disableLayoutState() { m_layoutStateDisableCount++; }
+    void enableLayoutState() { ASSERT(m_layoutStateDisableCount > 0); m_layoutStateDisableCount--; }
+
     size_t getRetainedWidgets(Vector<RenderWidget*>&);
     void releaseWidgets(Vector<RenderWidget*>&);
     
     friend class LayoutStateMaintainer;
+    friend class LayoutStateDisabler;
         
 protected:
@@ -322 +324 @@
 };
 
+class LayoutStateDisabler {
+    WTF_MAKE_NONCOPYABLE(LayoutStateDisabler);
+public:
+    LayoutStateDisabler(RenderView* view)
+        : m_view(view)
+    {
+        if (m_view)
+            m_view->disableLayoutState();
+    }
+
+    ~LayoutStateDisabler()
+    {
+        if (m_view)
+            m_view->enableLayoutState();
+    }
+private:
+    RenderView* m_view;
+};
+
 } // namespace WebCore
 

trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp

@@ -104 +104 @@
 
     // Arbitrary affine transforms are incompatible with LayoutState.
-    view()->disableLayoutState();
+    LayoutStateDisabler layoutStateDisabler(view());
 
     bool needsLayout = selfNeedsLayout();
@@ -129 +129 @@
     repainter.repaintAfterLayout();
 
-    view()->enableLayoutState();
     setNeedsLayout(false);
 }