Even more PassRef<RenderStyle>!
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Oct 2013 10:42:20 +0000 (10:42 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Oct 2013 10:42:20 +0000 (10:42 +0000)
<https://webkit.org/b/123147>

Convert more of the WebCore code to use PassRef for RenderStyle
in places where they are known to be non-null.

Re-landing this without region styling since that caused some
assertions last time.

Reviewed by Antti Koivisto.

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

14 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ElementRareData.h
Source/WebCore/html/HTMLTitleElement.cpp
Source/WebCore/page/animation/AnimationController.cpp
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/page/animation/CompositeAnimation.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderRegion.cpp
Source/WebCore/style/StyleResolveTree.cpp

index 042539a..9a39724 100644 (file)
@@ -1,3 +1,16 @@
+2013-10-22  Andreas Kling  <akling@apple.com>
+
+        Even more PassRef<RenderStyle>!
+        <https://webkit.org/b/123147>
+
+        Convert more of the WebCore code to use PassRef for RenderStyle
+        in places where they are known to be non-null.
+
+        Re-landing this without region styling since that caused some
+        assertions last time.
+
+        Reviewed by Antti Koivisto.
+
 2013-10-22  Zoltan Horvath  <zoltan@webkit.org>
 
         Refactor LineBreaker::nextSegmentBreak, add BreakingContext that holds all its state
index b7f6f27..83bd3c6 100644 (file)
@@ -1856,15 +1856,12 @@ void Document::updateLayoutIgnorePendingStylesheets()
     m_ignorePendingStylesheets = oldIgnore;
 }
 
-PassRefPtr<RenderStyle> Document::styleForElementIgnoringPendingStylesheets(Element* element)
+PassRef<RenderStyle> Document::styleForElementIgnoringPendingStylesheets(Element* element)
 {
     ASSERT_ARG(element, &element->document() == this);
 
-    bool oldIgnore = m_ignorePendingStylesheets;
-    m_ignorePendingStylesheets = true;
-    RefPtr<RenderStyle> style = ensureStyleResolver().styleForElement(element, element->parentNode() ? element->parentNode()->computedStyle() : nullptr);
-    m_ignorePendingStylesheets = oldIgnore;
-    return style.release();
+    TemporaryChange<bool> change(m_ignorePendingStylesheets, true);
+    return ensureStyleResolver().styleForElement(element, element->parentNode() ? element->parentNode()->computedStyle() : nullptr);
 }
 
 bool Document::isPageBoxVisible(int pageIndex)
index b858778..03b12fe 100644 (file)
@@ -527,7 +527,7 @@ public:
     void updateStyleIfNeeded();
     void updateLayout();
     void updateLayoutIgnorePendingStylesheets();
-    PassRefPtr<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
+    PassRef<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
 
     // Returns true if page box (margin boxes and page borders) is visible.
     bool isPageBoxVisible(int pageIndex);
index 067370c..7356658 100644 (file)
@@ -1477,11 +1477,11 @@ void Element::lazyAttach(ShouldSetAttached shouldSetAttached)
     markAncestorsWithChildNeedsStyleRecalc();
 }
 
-PassRefPtr<RenderStyle> Element::styleForRenderer()
+PassRef<RenderStyle> Element::styleForRenderer()
 {
     if (hasCustomStyleResolveCallbacks()) {
         if (RefPtr<RenderStyle> style = customStyleForRenderer())
-            return style.release();
+            return style.releaseNonNull();
     }
 
     return document().ensureStyleResolver().styleForElement(this);
index c91804f..93ce070 100644 (file)
@@ -520,7 +520,7 @@ public:
     
     virtual bool isSpellCheckingEnabled() const;
 
-    PassRefPtr<RenderStyle> styleForRenderer();
+    PassRef<RenderStyle> styleForRenderer();
 
     RenderRegion* renderRegion() const;
 
index fe2a825..bbe26f8 100644 (file)
@@ -102,7 +102,7 @@ public:
     void setAttributeMap(PassOwnPtr<NamedNodeMap> attributeMap) { m_attributeMap = attributeMap; }
 
     RenderStyle* computedStyle() const { return m_computedStyle.get(); }
-    void setComputedStyle(PassRefPtr<RenderStyle> computedStyle) { m_computedStyle = computedStyle; }
+    void setComputedStyle(PassRef<RenderStyle> computedStyle) { m_computedStyle = std::move(computedStyle); }
 
     ClassList* classList() const { return m_classList.get(); }
     void setClassList(OwnPtr<ClassList> classList) { m_classList = std::move(classList); }
@@ -225,7 +225,7 @@ inline void ElementRareData::setAfterPseudoElement(PassRefPtr<PseudoElement> pse
 
 inline void ElementRareData::resetComputedStyle()
 {
-    setComputedStyle(0);
+    m_computedStyle = nullptr;
     setStyleAffectedByEmpty(false);
     setChildIndex(0);
 }
index 7323d3a..a62d9a3 100644 (file)
@@ -82,10 +82,12 @@ String HTMLTitleElement::text() const
 StringWithDirection HTMLTitleElement::textWithDirection()
 {
     TextDirection direction = LTR;
-    if (RenderStyle* style = computedStyle())
-        direction = style->direction();
-    else if (RefPtr<RenderStyle> style = styleForRenderer())
-        direction = style->direction();
+    if (RenderStyle* computedStyle = this->computedStyle())
+        direction = computedStyle->direction();
+    else {
+        Ref<RenderStyle> style(styleForRenderer());
+        direction = style.get().direction();
+    }
     return StringWithDirection(text(), direction);
 }
 
index 6249a72..2e4e92c 100644 (file)
@@ -520,9 +520,7 @@ PassRef<RenderStyle> AnimationController::updateAnimations(RenderElement& render
     Ref<RenderStyle> newStyleBeforeAnimation(std::move(newStyle));
 
     CompositeAnimation& rendererAnimations = m_data->ensureCompositeAnimation(&renderer);
-
-    // FIXME: This could be a PassRef<RenderStyle>.
-    RefPtr<RenderStyle> blendedStyle = rendererAnimations.animate(&renderer, oldStyle, &newStyleBeforeAnimation.get());
+    auto blendedStyle = rendererAnimations.animate(renderer, oldStyle, newStyleBeforeAnimation.get());
 
     if (renderer.parent() || newStyleBeforeAnimation->animations() || (oldStyle && oldStyle->animations())) {
         m_data->updateAnimationTimerForRenderer(&renderer);
@@ -531,14 +529,14 @@ PassRef<RenderStyle> AnimationController::updateAnimations(RenderElement& render
 #endif
     }
 
-    if (blendedStyle != &newStyleBeforeAnimation.get()) {
+    if (&blendedStyle.get() != &newStyleBeforeAnimation.get()) {
         // If the animations/transitions change opacity or transform, we need to update
         // the style to impose the stacking rules. Note that this is also
         // done in StyleResolver::adjustRenderStyle().
-        if (blendedStyle->hasAutoZIndex() && (blendedStyle->opacity() < 1.0f || blendedStyle->hasTransform()))
-            blendedStyle->setZIndex(0);
+        if (blendedStyle.get().hasAutoZIndex() && (blendedStyle.get().opacity() < 1.0f || blendedStyle.get().hasTransform()))
+            blendedStyle.get().setZIndex(0);
     }
-    return blendedStyle.releaseNonNull();
+    return blendedStyle;
 }
 
 PassRefPtr<RenderStyle> AnimationController::getAnimatedStyleForRenderer(RenderElement* renderer)
index 58d2719..e18779e 100644 (file)
@@ -295,13 +295,13 @@ void CompositeAnimation::updateKeyframeAnimations(RenderElement* renderer, Rende
         m_keyframeAnimations.remove(animsToBeRemoved[j]);
 }
 
-PassRefPtr<RenderStyle> CompositeAnimation::animate(RenderElement* renderer, RenderStyle* currentStyle, RenderStyle* targetStyle)
+PassRef<RenderStyle> CompositeAnimation::animate(RenderElement& renderer, RenderStyle* currentStyle, RenderStyle& targetStyle)
 {
     RefPtr<RenderStyle> resultStyle;
 
     // We don't do any transitions if we don't have a currentStyle (on startup).
-    updateTransitions(renderer, currentStyle, targetStyle);
-    updateKeyframeAnimations(renderer, currentStyle, targetStyle);
+    updateTransitions(&renderer, currentStyle, &targetStyle);
+    updateKeyframeAnimations(&renderer, currentStyle, &targetStyle);
     m_keyframeAnimations.checkConsistency();
 
     if (currentStyle) {
@@ -311,7 +311,7 @@ PassRefPtr<RenderStyle> CompositeAnimation::animate(RenderElement* renderer, Ren
             CSSPropertyTransitionsMap::const_iterator end = m_transitions.end();
             for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin(); it != end; ++it) {
                 if (ImplicitAnimation* anim = it->value.get())
-                    anim->animate(this, renderer, currentStyle, targetStyle, resultStyle);
+                    anim->animate(this, &renderer, currentStyle, &targetStyle, resultStyle);
             }
         }
     }
@@ -321,10 +321,10 @@ PassRefPtr<RenderStyle> CompositeAnimation::animate(RenderElement* renderer, Ren
     for (Vector<AtomicStringImpl*>::const_iterator it = m_keyframeAnimationOrderMap.begin(); it != m_keyframeAnimationOrderMap.end(); ++it) {
         RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(*it);
         if (keyframeAnim)
-            keyframeAnim->animate(this, renderer, currentStyle, targetStyle, resultStyle);
+            keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, resultStyle);
     }
 
-    return resultStyle ? resultStyle.release() : targetStyle;
+    return resultStyle ? resultStyle.releaseNonNull() : targetStyle;
 }
 
 PassRefPtr<RenderStyle> CompositeAnimation::getAnimatedStyle() const
index 8e18774..48c679e 100644 (file)
@@ -55,7 +55,7 @@ public:
     
     void clearRenderer();
 
-    PassRefPtr<RenderStyle> animate(RenderElement*, RenderStyle* currentStyle, RenderStyle* targetStyle);
+    PassRef<RenderStyle> animate(RenderElement&, RenderStyle* currentStyle, RenderStyle& targetStyle);
     PassRefPtr<RenderStyle> getAnimatedStyle() const;
 
     double timeToNextService() const;
index 8c4c38d..6dee372 100644 (file)
@@ -128,13 +128,13 @@ RenderElement* RenderElement::createFor(Element& element, RenderStyle& style)
         // RenderImageResourceStyleImage requires a style being present on the image but we don't want to
         // trigger a style change now as the node is not fully attached. Moving this code to style change
         // doesn't make sense as it should be run once at renderer creation.
-        image->setStyleInternal(&style);
+        image->setStyleInternal(style);
         if (const StyleImage* styleImage = static_cast<const ImageContentData*>(contentData)->image()) {
             image->setImageResource(RenderImageResourceStyleImage::create(const_cast<StyleImage*>(styleImage)));
             image->setIsGeneratedContent();
         } else
             image->setImageResource(RenderImageResource::create());
-        image->setStyleInternal(0);
+        image->clearStyleInternal();
         return image;
     }
 
index 57f35ac..6d9bbe0 100644 (file)
@@ -103,7 +103,8 @@ public:
 
     // Updates only the local style ptr of the object. Does not update the state of the object,
     // and so only should be called when the style is known not to have changed (or from setStyle).
-    void setStyleInternal(PassRefPtr<RenderStyle> style) { m_style = style; }
+    void setStyleInternal(PassRef<RenderStyle> style) { m_style = std::move(style); }
+    void clearStyleInternal() { m_style = nullptr; }
 
 protected:
     enum BaseTypeFlags {
index 7dd1c90..ed5a74e 100644 (file)
@@ -540,7 +540,7 @@ void RenderRegion::restoreRegionObjectsOriginalStyle()
         RefPtr<RenderStyle> objectRegionStyle = object->style();
         RefPtr<RenderStyle> objectOriginalStyle = iter->value.style;
         if (object->isRenderElement())
-            toRenderElement(object)->setStyleInternal(objectOriginalStyle);
+            toRenderElement(object)->setStyleInternal(*objectOriginalStyle);
 
         bool shouldCacheRegionStyle = iter->value.cached;
         if (!shouldCacheRegionStyle) {
@@ -621,7 +621,7 @@ void RenderRegion::setObjectStyleInRegion(RenderObject* object, PassRefPtr<Rende
 
     RefPtr<RenderStyle> objectOriginalStyle = object->style();
     if (object->isRenderElement())
-        toRenderElement(object)->setStyleInternal(styleInRegion);
+        toRenderElement(object)->setStyleInternal(*styleInRegion);
 
     if (object->isBoxModelObject() && !object->hasBoxDecorations()) {
         bool hasBoxDecorations = object->isTableCell()
index b856602..065165e 100644 (file)
@@ -606,11 +606,11 @@ static Change resolveLocal(Element& current, Change inheritedChange)
 
     if (RenderElement* renderer = current.renderer()) {
         if (localChange != NoChange || pseudoStyleCacheIsInvalid(renderer, newStyle.get()) || (inheritedChange == Force && renderer->requiresForcedStyleRecalcPropagation()) || current.styleChangeType() == SyntheticStyleChange)
-            renderer->setAnimatableStyle(*newStyle.get());
+            renderer->setAnimatableStyle(*newStyle);
         else if (current.needsStyleRecalc()) {
             // Although no change occurred, we use the new style so that the cousin style sharing code won't get
             // fooled into believing this style is the same.
-            renderer->setStyleInternal(newStyle.get());
+            renderer->setStyleInternal(*newStyle);
         }
     }