RenderElement::m_style should be a Ref.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 28 Oct 2013 15:48:44 +0000 (15:48 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 28 Oct 2013 15:48:44 +0000 (15:48 +0000)
<https://webkit.org/b/123401>

Source/WebCore:

Made RenderElement::m_style a Ref. This codifies the fact that it
can never be null after construction.

Removed a couple of unnecessary null checks that were exposed as
compilation failures.

Reviewed by Antti Koivisto.

Source/WTF:

Added a Ref::replace() so we can Indiana Jones the new style in
RenderElement::setStyle() while keeping a handle on the old style
for a while longer.

Reviewed by Antti Koivisto.

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

Source/WTF/ChangeLog
Source/WTF/wtf/Ref.h
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h

index 2dc49fd..fc94090 100644 (file)
@@ -1,3 +1,14 @@
+2013-10-28  Andreas Kling  <akling@apple.com>
+
+        RenderElement::m_style should be a Ref.
+        <https://webkit.org/b/123401>
+
+        Added a Ref::replace() so we can Indiana Jones the new style in
+        RenderElement::setStyle() while keeping a handle on the old style
+        for a while longer.
+
+        Reviewed by Antti Koivisto.
+
 2013-10-28  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix make distcheck.
index 6a0d971..6ff7c82 100644 (file)
@@ -60,10 +60,19 @@ public:
     const T& get() const { return *m_ptr; }
     T& get() { return *m_ptr; }
 
+    template<typename U> PassRef<T> replace(PassRef<U>) WARN_UNUSED_RETURN;
+
 private:
     T* m_ptr;
 };
 
+template<typename T> template<typename U> inline PassRef<T> Ref<T>::replace(PassRef<U> reference)
+{
+    auto oldReference = adoptRef(*m_ptr);
+    m_ptr = &reference.leakRef();
+    return oldReference;
+}
+
 } // namespace WTF
 
 using WTF::Ref;
index dc7c5a3..dabb32e 100644 (file)
@@ -1,3 +1,16 @@
+2013-10-28  Andreas Kling  <akling@apple.com>
+
+        RenderElement::m_style should be a Ref.
+        <https://webkit.org/b/123401>
+
+        Made RenderElement::m_style a Ref. This codifies the fact that it
+        can never be null after construction.
+
+        Removed a couple of unnecessary null checks that were exposed as
+        compilation failures.
+
+        Reviewed by Antti Koivisto.
+
 2013-10-28  Bastien Nocera <hadess@hadess.net>
 
         Name all the GLib timeout sources
index 010430e..6da8210 100644 (file)
@@ -392,7 +392,7 @@ void RenderElement::initializeStyle()
 
 void RenderElement::setStyle(PassRef<RenderStyle> style)
 {
-    if (m_style == &style.get()) {
+    if (&m_style.get() == &style.get()) {
 #if USE(ACCELERATED_COMPOSITING)
         // We need to run through adjustStyleDifference() for iframes, plugins, and canvas so
         // style sharing is disabled for them. That should ensure that we never hit this code path.
@@ -410,19 +410,18 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)
     diff = adjustStyleDifference(diff, contextSensitiveProperties);
 
     styleWillChange(diff, style.get());
-    
-    RefPtr<RenderStyle> oldStyle = m_style.release();
-    m_style = std::move(style);
 
-    updateFillImages(oldStyle ? oldStyle->backgroundLayers() : nullptr, m_style->backgroundLayers());
-    updateFillImages(oldStyle ? oldStyle->maskLayers() : nullptr, m_style->maskLayers());
+    Ref<RenderStyle> oldStyle(m_style.replace(std::move(style)));
+
+    updateFillImages(oldStyle.get().backgroundLayers(), m_style->backgroundLayers());
+    updateFillImages(oldStyle.get().maskLayers(), m_style->maskLayers());
 
-    updateImage(oldStyle ? oldStyle->borderImage().image() : nullptr, m_style->borderImage().image());
-    updateImage(oldStyle ? oldStyle->maskBoxImage().image() : nullptr, m_style->maskBoxImage().image());
+    updateImage(oldStyle.get().borderImage().image(), m_style->borderImage().image());
+    updateImage(oldStyle.get().maskBoxImage().image(), m_style->maskBoxImage().image());
 
 #if ENABLE(CSS_SHAPES)
-    updateShapeImage(oldStyle ? oldStyle->shapeInside() : nullptr, m_style->shapeInside());
-    updateShapeImage(oldStyle ? oldStyle->shapeOutside() : nullptr, m_style->shapeOutside());
+    updateShapeImage(oldStyle.get().shapeInside(), m_style->shapeInside());
+    updateShapeImage(oldStyle.get().shapeOutside(), m_style->shapeOutside());
 #endif
 
     // We need to ensure that view->maximalOutlineSize() is valid for any repaints that happen
@@ -432,12 +431,12 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)
 
     bool doesNotNeedLayout = !parent();
 
-    styleDidChange(diff, oldStyle.get());
+    styleDidChange(diff, &oldStyle.get());
 
     // Text renderers use their parent style. Notify them about the change.
     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
         if (child->isText())
-            toRenderText(child)->styleDidChange(diff, oldStyle.get());
+            toRenderText(child)->styleDidChange(diff, &oldStyle.get());
     }
 
     // FIXME: |this| might be destroyed here. This can currently happen for a RenderTextFragment when
@@ -455,9 +454,9 @@ void RenderElement::setStyle(PassRef<RenderStyle> style)
         if (updatedDiff == StyleDifferenceLayout)
             setNeedsLayoutAndPrefWidthsRecalc();
         else if (updatedDiff == StyleDifferenceLayoutPositionedMovementOnly)
-            setNeedsPositionedMovementLayout(oldStyle.get());
+            setNeedsPositionedMovementLayout(&oldStyle.get());
         else if (updatedDiff == StyleDifferenceSimplifiedLayoutAndPositionedMovement) {
-            setNeedsPositionedMovementLayout(oldStyle.get());
+            setNeedsPositionedMovementLayout(&oldStyle.get());
             setNeedsSimplifiedNormalFlowLayout();
         } else if (updatedDiff == StyleDifferenceSimplifiedLayout)
             setNeedsSimplifiedNormalFlowLayout();
@@ -950,7 +949,7 @@ void RenderElement::styleDidChange(StyleDifference diff, const RenderStyle* oldS
         return;
     
     if (diff == StyleDifferenceLayout || diff == StyleDifferenceSimplifiedLayout) {
-        RenderCounter::rendererStyleChanged(this, oldStyle, m_style.get());
+        RenderCounter::rendererStyleChanged(this, oldStyle, &m_style.get());
 
         // If the object already needs layout, then setNeedsLayout won't do
         // any work. But if the containing block has changed, then we may need
@@ -1015,7 +1014,7 @@ void RenderElement::willBeRemovedFromTree()
     }
 
     bool repaintFixedBackgroundsOnScroll = shouldRepaintFixedBackgroundsOnScroll();
-    if (repaintFixedBackgroundsOnScroll && m_style && m_style->hasFixedBackgroundImage())
+    if (repaintFixedBackgroundsOnScroll && m_style->hasFixedBackgroundImage())
         view().frameView().removeSlowRepaintObject(this);
 
     if (isOutOfFlowPositioned() && parent()->childrenInline())
index cfba369..c979114 100644 (file)
@@ -35,7 +35,7 @@ public:
 
     bool hasInitializedStyle() const { return m_hasInitializedStyle; }
 
-    RenderStyle* style() const { return m_style.get(); }
+    RenderStyle* style() const { return const_cast<RenderStyle*>(&m_style.get()); }
     RenderStyle* firstLineStyle() const;
 
     void initializeStyle();
@@ -170,7 +170,7 @@ private:
     RenderObject* m_firstChild;
     RenderObject* m_lastChild;
 
-    RefPtr<RenderStyle> m_style;
+    Ref<RenderStyle> m_style;
 
     // FIXME: Get rid of this hack.
     // Store state between styleWillChange and styleDidChange