suspend/resumeWidgetHierarchyUpdates should be a RAII object
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Sep 2012 20:34:35 +0000 (20:34 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Sep 2012 20:34:35 +0000 (20:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=96706

Reviewed by Simon Fraser.

Source/WebCore:

Replaced suspendWidgetHierarchyUpdates and resumeWidgetHierarchyUpdates by WidgetHierarchyUpdatesSuspensionScope.

* WebCore.exp.in: Export new symbols.
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeChild):
(WebCore::ContainerNode::removeChildren):
* dom/Document.cpp:
(WebCore::Document::recalcStyle):
* dom/Element.cpp:
(WebCore::Element::attach):
(WebCore::Element::detach):
* page/mac/EventHandlerMac.mm:
(WebCore::EventHandler::passMouseDownEventToWidget):
* rendering/RenderWidget.cpp:
(WebCore):
(WebCore::WidgetHierarchyUpdatesSuspensionScope::widgetNewParentMap):
(WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets):
(WebCore::moveWidgetToParentSoon):
* rendering/RenderWidget.h:
(WidgetHierarchyUpdatesSuspensionScope):
(WebCore::WidgetHierarchyUpdatesSuspensionScope::WidgetHierarchyUpdatesSuspensionScope):
(WebCore::WidgetHierarchyUpdatesSuspensionScope::~WidgetHierarchyUpdatesSuspensionScope):
(WebCore::WidgetHierarchyUpdatesSuspensionScope::isSuspended):
(WebCore::WidgetHierarchyUpdatesSuspensionScope::scheduleWidgetToMove):
(WebCore):
(RenderWidget):

Source/WebKit/mac:

* WebView/WebHTMLView.mm:
(-[WebHTMLView _invalidateGStatesForTree]):

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

Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/dom/ContainerNode.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Element.cpp
Source/WebCore/page/mac/EventHandlerMac.mm
Source/WebCore/rendering/RenderWidget.cpp
Source/WebCore/rendering/RenderWidget.h
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebHTMLView.mm

index 0247dd6..4c4f950 100644 (file)
@@ -1,3 +1,37 @@
+2012-09-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        suspend/resumeWidgetHierarchyUpdates should be a RAII object
+        https://bugs.webkit.org/show_bug.cgi?id=96706
+
+        Reviewed by Simon Fraser.
+
+        Replaced suspendWidgetHierarchyUpdates and resumeWidgetHierarchyUpdates by WidgetHierarchyUpdatesSuspensionScope.
+
+        * WebCore.exp.in: Export new symbols.
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::removeChild):
+        (WebCore::ContainerNode::removeChildren):
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle):
+        * dom/Element.cpp:
+        (WebCore::Element::attach):
+        (WebCore::Element::detach):
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::EventHandler::passMouseDownEventToWidget):
+        * rendering/RenderWidget.cpp:
+        (WebCore):
+        (WebCore::WidgetHierarchyUpdatesSuspensionScope::widgetNewParentMap):
+        (WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets):
+        (WebCore::moveWidgetToParentSoon):
+        * rendering/RenderWidget.h:
+        (WidgetHierarchyUpdatesSuspensionScope):
+        (WebCore::WidgetHierarchyUpdatesSuspensionScope::WidgetHierarchyUpdatesSuspensionScope):
+        (WebCore::WidgetHierarchyUpdatesSuspensionScope::~WidgetHierarchyUpdatesSuspensionScope):
+        (WebCore::WidgetHierarchyUpdatesSuspensionScope::isSuspended):
+        (WebCore::WidgetHierarchyUpdatesSuspensionScope::scheduleWidgetToMove):
+        (WebCore):
+        (RenderWidget):
+
 2012-09-24  Otto Derek Cheung  <otcheung@rim.com>
 
         [BlackBerry] Reverting implementation for 407 error pages
index 58c504d..a196883 100644 (file)
@@ -186,8 +186,6 @@ __ZN7WebCore12PrintContextC1EPNS_5FrameE
 __ZN7WebCore12PrintContextD1Ev
 __ZNK7WebCore12RenderObject16repaintRectangleERKNS_20FractionalLayoutRectEb
 __ZN7WebCore12RenderObject19scrollRectToVisibleERKNS_20FractionalLayoutRectERKNS_15ScrollAlignmentES6_
-__ZN7WebCore12RenderWidget28resumeWidgetHierarchyUpdatesEv
-__ZN7WebCore12RenderWidget29suspendWidgetHierarchyUpdatesEv
 __ZN7WebCore12SharedBuffer10wrapCFDataEPK8__CFData
 __ZN7WebCore12SharedBuffer10wrapNSDataEP6NSData
 __ZN7WebCore12SharedBuffer12createNSDataEv
@@ -638,6 +636,8 @@ __ZN7WebCore31CrossOriginPreflightResultCache5emptyEv
 __ZN7WebCore31CrossOriginPreflightResultCache6sharedEv
 __ZN7WebCore32plainTextToMallocAllocatedBufferEPKNS_5RangeERjbNS_20TextIteratorBehaviorE
 __ZN7WebCore33stripLeadingAndTrailingHTMLSpacesERKN3WTF6StringE
+__ZN7WebCore37WidgetHierarchyUpdatesSuspensionScope11moveWidgetsEv
+__ZN7WebCore37WidgetHierarchyUpdatesSuspensionScope35s_widgetHierarchyUpdateSuspendCountE
 __ZN7WebCore3macERKNS_10CredentialE
 __ZN7WebCore3macERKNS_23AuthenticationChallengeE
 __ZN7WebCore40restrictMinimumScaleFactorToViewportSizeERNS_18ViewportAttributesENS_7IntSizeE
index cd118f1..dafee60 100644 (file)
@@ -419,15 +419,15 @@ bool ContainerNode::removeChild(Node* oldChild, ExceptionCode& ec)
         return false;
     }
 
-    RenderWidget::suspendWidgetHierarchyUpdates();
-
-    Node* prev = child->previousSibling();
-    Node* next = child->nextSibling();
-    removeBetween(prev, next, child.get());
-    childrenChanged(false, prev, next, -1);
-    ChildNodeRemovalNotifier(this).notify(child.get());
-
-    RenderWidget::resumeWidgetHierarchyUpdates();
+    {
+        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+
+        Node* prev = child->previousSibling();
+        Node* next = child->nextSibling();
+        removeBetween(prev, next, child.get());
+        childrenChanged(false, prev, next, -1);
+        ChildNodeRemovalNotifier(this).notify(child.get());
+    }
     dispatchSubtreeModifiedEvent();
 
     return child;
@@ -497,49 +497,50 @@ void ContainerNode::removeChildren()
     // and remove... e.g. stop loading frames, fire unload events.
     willRemoveChildren(protect.get());
 
-    RenderWidget::suspendWidgetHierarchyUpdates();
-    forbidEventDispatch();
     Vector<RefPtr<Node>, 10> removedChildren;
-    removedChildren.reserveInitialCapacity(childNodeCount());
-    while (RefPtr<Node> n = m_firstChild) {
-        Node* next = n->nextSibling();
-
-        // Remove the node from the tree before calling detach or removedFromDocument (4427024, 4129744).
-        // removeChild() does this after calling detach(). There is no explanation for
-        // this discrepancy between removeChild() and its optimized version removeChildren().
-        n->setPreviousSibling(0);
-        n->setNextSibling(0);
-        n->setParentOrHostNode(0);
-        document()->adoptIfNeeded(n.get());
-
-        m_firstChild = next;
-        if (n == m_lastChild)
-            m_lastChild = 0;
-        removedChildren.append(n.release());
-    }
+    {
+        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+        forbidEventDispatch();
+        removedChildren.reserveInitialCapacity(childNodeCount());
+        while (RefPtr<Node> n = m_firstChild) {
+            Node* next = n->nextSibling();
+
+            // Remove the node from the tree before calling detach or removedFromDocument (4427024, 4129744).
+            // removeChild() does this after calling detach(). There is no explanation for
+            // this discrepancy between removeChild() and its optimized version removeChildren().
+            n->setPreviousSibling(0);
+            n->setNextSibling(0);
+            n->setParentOrHostNode(0);
+            document()->adoptIfNeeded(n.get());
+
+            m_firstChild = next;
+            if (n == m_lastChild)
+                m_lastChild = 0;
+            removedChildren.append(n.release());
+        }
 
-    size_t removedChildrenCount = removedChildren.size();
-    size_t i;
-
-    // Detach the nodes only after properly removed from the tree because
-    // a. detaching requires a proper DOM tree (for counters and quotes for
-    // example) and during the previous loop the next sibling still points to
-    // the node being removed while the node being removed does not point back
-    // and does not point to the same parent as its next sibling.
-    // b. destroying Renderers of standalone nodes is sometimes faster.
-    for (i = 0; i < removedChildrenCount; ++i) {
-        Node* removedChild = removedChildren[i].get();
-        if (removedChild->attached())
-            removedChild->detach();
-    }
+        size_t removedChildrenCount = removedChildren.size();
+        size_t i;
+
+        // Detach the nodes only after properly removed from the tree because
+        // a. detaching requires a proper DOM tree (for counters and quotes for
+        // example) and during the previous loop the next sibling still points to
+        // the node being removed while the node being removed does not point back
+        // and does not point to the same parent as its next sibling.
+        // b. destroying Renderers of standalone nodes is sometimes faster.
+        for (i = 0; i < removedChildrenCount; ++i) {
+            Node* removedChild = removedChildren[i].get();
+            if (removedChild->attached())
+                removedChild->detach();
+        }
 
-    childrenChanged(false, 0, 0, -static_cast<int>(removedChildrenCount));
+        childrenChanged(false, 0, 0, -static_cast<int>(removedChildrenCount));
 
-    for (i = 0; i < removedChildrenCount; ++i)
-        ChildNodeRemovalNotifier(this).notify(removedChildren[i].get());
+        for (i = 0; i < removedChildrenCount; ++i)
+            ChildNodeRemovalNotifier(this).notify(removedChildren[i].get());
 
-    allowEventDispatch();
-    RenderWidget::resumeWidgetHierarchyUpdates();
+        allowEventDispatch();
+    }
 
     dispatchSubtreeModifiedEvent();
 }
index 3fa6b41..4ed5d0e 100644 (file)
@@ -1832,65 +1832,66 @@ void Document::recalcStyle(StyleChange change)
 
     m_inStyleRecalc = true;
     suspendPostAttachCallbacks();
-    RenderWidget::suspendWidgetHierarchyUpdates();
-    
-    RefPtr<FrameView> frameView = view();
-    if (frameView) {
-        frameView->pauseScheduledEvents();
-        frameView->beginDeferredRepaints();
-    }
+    {
+        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
 
-    ASSERT(!renderer() || renderArena());
-    if (!renderer() || !renderArena())
-        goto bail_out;
+        RefPtr<FrameView> frameView = view();
+        if (frameView) {
+            frameView->pauseScheduledEvents();
+            frameView->beginDeferredRepaints();
+        }
 
-    if (m_pendingStyleRecalcShouldForce)
-        change = Force;
+        ASSERT(!renderer() || renderArena());
+        if (!renderer() || !renderArena())
+            goto bailOut;
 
-    // Recalculating the root style (on the document) is not needed in the common case.
-    if ((change == Force) || (shouldDisplaySeamlesslyWithParent() && (change >= Inherit))) {
-        // style selector may set this again during recalc
-        m_hasNodesWithPlaceholderStyle = false;
-        
-        RefPtr<RenderStyle> documentStyle = StyleResolver::styleForDocument(this, m_styleResolver ? m_styleResolver->fontSelector() : 0);
-        StyleChange ch = Node::diff(documentStyle.get(), renderer()->style(), this);
-        if (ch != NoChange)
-            renderer()->setStyle(documentStyle.release());
-    }
+        if (m_pendingStyleRecalcShouldForce)
+            change = Force;
 
-    for (Node* n = firstChild(); n; n = n->nextSibling()) {
-        if (!n->isElementNode())
-            continue;
-        Element* element = static_cast<Element*>(n);
-        if (change >= Inherit || element->childNeedsStyleRecalc() || element->needsStyleRecalc())
-            element->recalcStyle(change);
-    }
+        // Recalculating the root style (on the document) is not needed in the common case.
+        if ((change == Force) || (shouldDisplaySeamlesslyWithParent() && (change >= Inherit))) {
+            // style selector may set this again during recalc
+            m_hasNodesWithPlaceholderStyle = false;
+            
+            RefPtr<RenderStyle> documentStyle = StyleResolver::styleForDocument(this, m_styleResolver ? m_styleResolver->fontSelector() : 0);
+            StyleChange ch = Node::diff(documentStyle.get(), renderer()->style(), this);
+            if (ch != NoChange)
+                renderer()->setStyle(documentStyle.release());
+        }
 
-#if USE(ACCELERATED_COMPOSITING)
-    if (view()) {
-        bool layoutPending = view()->layoutPending() || renderer()->needsLayout();
-        // If we didn't update compositing layers because of layout(), we need to do so here.
-        if (!layoutPending)
-            view()->updateCompositingLayersAfterStyleChange();
-    }
-#endif
+        for (Node* n = firstChild(); n; n = n->nextSibling()) {
+            if (!n->isElementNode())
+                continue;
+            Element* element = static_cast<Element*>(n);
+            if (change >= Inherit || element->childNeedsStyleRecalc() || element->needsStyleRecalc())
+                element->recalcStyle(change);
+        }
 
-bail_out:
-    clearNeedsStyleRecalc();
-    clearChildNeedsStyleRecalc();
-    unscheduleStyleRecalc();
+    #if USE(ACCELERATED_COMPOSITING)
+        if (view()) {
+            bool layoutPending = view()->layoutPending() || renderer()->needsLayout();
+            // If we didn't update compositing layers because of layout(), we need to do so here.
+            if (!layoutPending)
+                view()->updateCompositingLayersAfterStyleChange();
+        }
+    #endif
 
-    m_inStyleRecalc = false;
-    
-    // Pseudo element removal and similar may only work with these flags still set. Reset them after the style recalc.
-    if (m_styleResolver)
-        resetCSSFeatureFlags();
+    bailOut:
+        clearNeedsStyleRecalc();
+        clearChildNeedsStyleRecalc();
+        unscheduleStyleRecalc();
 
-    if (frameView) {
-        frameView->resumeScheduledEvents();
-        frameView->endDeferredRepaints();
+        m_inStyleRecalc = false;
+
+        // Pseudo element removal and similar may only work with these flags still set. Reset them after the style recalc.
+        if (m_styleResolver)
+            resetCSSFeatureFlags();
+
+        if (frameView) {
+            frameView->resumeScheduledEvents();
+            frameView->endDeferredRepaints();
+        }
     }
-    RenderWidget::resumeWidgetHierarchyUpdates();
     resumePostAttachCallbacks();
 
     // If we wanted to call implicitClose() during recalcStyle, do so now that we're finished.
index 15804a3..ead9204 100644 (file)
@@ -999,7 +999,7 @@ void Element::removedFrom(ContainerNode* insertionPoint)
 void Element::attach()
 {
     suspendPostAttachCallbacks();
-    RenderWidget::suspendWidgetHierarchyUpdates();
+    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
 
     createRendererIfNeeded();
     StyleResolverParentPusher parentPusher(this);
@@ -1028,7 +1028,6 @@ void Element::attach()
         }
     }
 
-    RenderWidget::resumeWidgetHierarchyUpdates();
     resumePostAttachCallbacks();
 }
 
@@ -1042,7 +1041,7 @@ void Element::unregisterNamedFlowContentNode()
 
 void Element::detach()
 {
-    RenderWidget::suspendWidgetHierarchyUpdates();
+    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
     unregisterNamedFlowContentNode();
     cancelFocusAppearanceUpdate();
     if (hasRareData()) {
@@ -1055,8 +1054,6 @@ void Element::detach()
         shadow->detach();
     }
     ContainerNode::detach();
-
-    RenderWidget::resumeWidgetHierarchyUpdates();
 }
 
 bool Element::pseudoStyleCacheIsInvalid(const RenderStyle* currentStyle, RenderStyle* newStyle)
index bd8ee47..d2020c7 100644 (file)
@@ -225,9 +225,10 @@ bool EventHandler::passMouseDownEventToWidget(Widget* pWidget)
     ASSERT(!m_sendingEventToSubview);
     m_sendingEventToSubview = true;
 
-    RenderWidget::suspendWidgetHierarchyUpdates();
-    [view mouseDown:currentNSEvent()];
-    RenderWidget::resumeWidgetHierarchyUpdates();
+    {
+        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+        [view mouseDown:currentNSEvent()];
+    }
 
     m_sendingEventToSubview = false;
     
index 9537edf..21d0c3a 100644 (file)
@@ -47,53 +47,42 @@ static HashMap<const Widget*, RenderWidget*>& widgetRendererMap()
     return *staticWidgetRendererMap;
 }
 
-static unsigned widgetHierarchyUpdateSuspendCount;
+unsigned WidgetHierarchyUpdatesSuspensionScope::s_widgetHierarchyUpdateSuspendCount = 0;
 
-typedef HashMap<RefPtr<Widget>, FrameView*> WidgetToParentMap;
-
-static WidgetToParentMap& widgetNewParentMap()
+WidgetHierarchyUpdatesSuspensionScope::WidgetToParentMap& WidgetHierarchyUpdatesSuspensionScope::widgetNewParentMap()
 {
     DEFINE_STATIC_LOCAL(WidgetToParentMap, map, ());
     return map;
 }
 
-void RenderWidget::suspendWidgetHierarchyUpdates()
+void WidgetHierarchyUpdatesSuspensionScope::moveWidgets()
 {
-    widgetHierarchyUpdateSuspendCount++;
-}
-
-void RenderWidget::resumeWidgetHierarchyUpdates()
-{
-    ASSERT(widgetHierarchyUpdateSuspendCount);
-    if (widgetHierarchyUpdateSuspendCount == 1) {
-        WidgetToParentMap map = widgetNewParentMap();
-        widgetNewParentMap().clear();
-        WidgetToParentMap::iterator end = map.end();
-        for (WidgetToParentMap::iterator it = map.begin(); it != end; ++it) {
-            Widget* child = it->first.get();
-            ScrollView* currentParent = child->parent();
-            FrameView* newParent = it->second;
-            if (newParent != currentParent) {
-                if (currentParent)
-                    currentParent->removeChild(child);
-                if (newParent)
-                    newParent->addChild(child);
-            }
+    WidgetToParentMap map = widgetNewParentMap();
+    widgetNewParentMap().clear();
+    WidgetToParentMap::iterator end = map.end();
+    for (WidgetToParentMap::iterator it = map.begin(); it != end; ++it) {
+        Widget* child = it->first.get();
+        ScrollView* currentParent = child->parent();
+        FrameView* newParent = it->second;
+        if (newParent != currentParent) {
+            if (currentParent)
+                currentParent->removeChild(child);
+            if (newParent)
+                newParent->addChild(child);
         }
     }
-    widgetHierarchyUpdateSuspendCount--;
 }
 
 static void moveWidgetToParentSoon(Widget* child, FrameView* parent)
 {
-    if (!widgetHierarchyUpdateSuspendCount) {
+    if (!WidgetHierarchyUpdatesSuspensionScope::isSuspended()) {
         if (parent)
             parent->addChild(child);
         else
             child->removeFromParent();
         return;
     }
-    widgetNewParentMap().set(child, parent);
+    WidgetHierarchyUpdatesSuspensionScope::scheduleWidgetToMove(child, parent);
 }
 
 RenderWidget::RenderWidget(Node* node)
index b7db3b8..d20123a 100644 (file)
 
 namespace WebCore {
 
+class WidgetHierarchyUpdatesSuspensionScope {
+public:
+    WidgetHierarchyUpdatesSuspensionScope()
+    {
+        s_widgetHierarchyUpdateSuspendCount++;
+    }
+    ~WidgetHierarchyUpdatesSuspensionScope()
+    {
+        ASSERT(s_widgetHierarchyUpdateSuspendCount);
+        if (s_widgetHierarchyUpdateSuspendCount == 1)
+            moveWidgets();
+        s_widgetHierarchyUpdateSuspendCount--;
+    }
+
+    static bool isSuspended() { return s_widgetHierarchyUpdateSuspendCount; }
+    static void scheduleWidgetToMove(Widget* widget, FrameView* frame) { widgetNewParentMap().set(widget, frame); }
+
+private:
+    typedef HashMap<RefPtr<Widget>, FrameView*> WidgetToParentMap;
+    static WidgetToParentMap& widgetNewParentMap();
+
+    void moveWidgets();
+
+    static unsigned s_widgetHierarchyUpdateSuspendCount;
+};
+    
 class RenderWidget : public RenderReplaced, private OverlapTestRequestClient {
 public:
     virtual ~RenderWidget();
@@ -42,9 +68,6 @@ public:
     IntRect windowClipRect() const;
 
     void notifyWidget(WidgetNotification);
-    
-    static void suspendWidgetHierarchyUpdates();
-    static void resumeWidgetHierarchyUpdates();
 
     RenderArena* ref() { ++m_refCount; return renderArena(); }
     void deref(RenderArena*);
index 1a1fc15..1601cfe 100644 (file)
@@ -1,3 +1,13 @@
+2012-09-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        suspend/resumeWidgetHierarchyUpdates should be a RAII object
+        https://bugs.webkit.org/show_bug.cgi?id=96706
+
+        Reviewed by Simon Fraser.
+
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView _invalidateGStatesForTree]):
+
 2012-09-21  Chris Rogers  <crogers@google.com>
 
         Add Web Audio support for deprecated/legacy APIs
index bfede8d..82716a6 100644 (file)
@@ -3376,9 +3376,8 @@ static void setMenuTargets(NSMenu* menu)
     // descendants, including plug-in views. This can result in calls out to plug-in code and back into
     // WebCore via JavaScript, which could normally mutate the NSView tree while it is being traversed.
     // Defer those mutations while descendants are being traveresed.
-    RenderWidget::suspendWidgetHierarchyUpdates();
+    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
     [super _invalidateGStatesForTree];
-    RenderWidget::resumeWidgetHierarchyUpdates();
 }
 
 - (BOOL)isFlipped