TextAutoSizingKey should use normal refcounting

https://bugs.webkit.org/show_bug.cgi?id=156893

Reviewed by Andreas Kling.

Get rid of special refcounting of style in favor of RefPtr. It also becomes a move-only type
to support future switch to non-refcounted RenderStyle.

Also general cleanups and modernization.

* dom/Document.cpp:
(WebCore::TextAutoSizingTraits::constructDeletedValue):
(WebCore::TextAutoSizingTraits::isDeletedValue):
(WebCore::Document::addAutoSizingNode):
(WebCore::Document::validateAutoSizingNodes):
(WebCore::Document::resetAutoSizingNodes):

    Adopt to being move-only.

* rendering/TextAutoSizing.cpp:
(WebCore::cloneRenderStyleWithState):
(WebCore::TextAutoSizingKey::TextAutoSizingKey):

    Clone the style for safety against mutations. Cloning is cheap.

(WebCore::TextAutoSizingValue::numNodes):
(WebCore::TextAutoSizingValue::adjustNodeSizes):
(WebCore::TextAutoSizingValue::reset):
(WebCore::TextAutoSizingKey::~TextAutoSizingKey): Deleted.
(WebCore::TextAutoSizingKey::operator=): Deleted.
(WebCore::TextAutoSizingKey::ref): Deleted.
(WebCore::TextAutoSizingKey::deref): Deleted.
* rendering/TextAutoSizing.h:
(WebCore::TextAutoSizingKey::TextAutoSizingKey):
(WebCore::TextAutoSizingKey::style):
(WebCore::TextAutoSizingKey::isDeleted):
(WebCore::operator==):
(WebCore::TextAutoSizingKey::doc): Deleted.
(WebCore::TextAutoSizingKey::isValidDoc): Deleted.
(WebCore::TextAutoSizingKey::isValidStyle): Deleted.
(WebCore::TextAutoSizingKey::deletedKeyDoc): Deleted.
(WebCore::TextAutoSizingKey::deletedKeyStyle): Deleted.

    m_doc member is not used for anything except deleted value comparisons. Replace it with a bit.


Canonical link: https://commits.webkit.org/175020@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199893 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,50 @@
+2016-04-22  Antti Koivisto  <antti@apple.com>
+
+        TextAutoSizingKey should use normal refcounting
+        https://bugs.webkit.org/show_bug.cgi?id=156893
+
+        Reviewed by Andreas Kling.
+
+        Get rid of special refcounting of style in favor of RefPtr. It also becomes a move-only type
+        to support future switch to non-refcounted RenderStyle.
+
+        Also general cleanups and modernization.
+
+        * dom/Document.cpp:
+        (WebCore::TextAutoSizingTraits::constructDeletedValue):
+        (WebCore::TextAutoSizingTraits::isDeletedValue):
+        (WebCore::Document::addAutoSizingNode):
+        (WebCore::Document::validateAutoSizingNodes):
+        (WebCore::Document::resetAutoSizingNodes):
+
+            Adopt to being move-only.
+
+        * rendering/TextAutoSizing.cpp:
+        (WebCore::cloneRenderStyleWithState):
+        (WebCore::TextAutoSizingKey::TextAutoSizingKey):
+
+            Clone the style for safety against mutations. Cloning is cheap.
+
+        (WebCore::TextAutoSizingValue::numNodes):
+        (WebCore::TextAutoSizingValue::adjustNodeSizes):
+        (WebCore::TextAutoSizingValue::reset):
+        (WebCore::TextAutoSizingKey::~TextAutoSizingKey): Deleted.
+        (WebCore::TextAutoSizingKey::operator=): Deleted.
+        (WebCore::TextAutoSizingKey::ref): Deleted.
+        (WebCore::TextAutoSizingKey::deref): Deleted.
+        * rendering/TextAutoSizing.h:
+        (WebCore::TextAutoSizingKey::TextAutoSizingKey):
+        (WebCore::TextAutoSizingKey::style):
+        (WebCore::TextAutoSizingKey::isDeleted):
+        (WebCore::operator==):
+        (WebCore::TextAutoSizingKey::doc): Deleted.
+        (WebCore::TextAutoSizingKey::isValidDoc): Deleted.
+        (WebCore::TextAutoSizingKey::isValidStyle): Deleted.
+        (WebCore::TextAutoSizingKey::deletedKeyDoc): Deleted.
+        (WebCore::TextAutoSizingKey::deletedKeyStyle): Deleted.
+
+            m_doc member is not used for anything except deleted value comparisons. Replace it with a bit.
+
 2016-04-22  Chris Dumez  <cdumez@apple.com>
 
         Crash under FontCache::purgeInactiveFontData()

Source/WebCore/dom/Document.cpp

@@ -428,12 +428,12 @@ uint64_t Document::s_globalTreeVersion = 0;
 #if ENABLE(IOS_TEXT_AUTOSIZING)
 void TextAutoSizingTraits::constructDeletedValue(TextAutoSizingKey& slot)
 {
-    new (&slot) TextAutoSizingKey(TextAutoSizingKey::deletedKeyStyle(), TextAutoSizingKey::deletedKeyDoc());
+    new (&slot) TextAutoSizingKey(TextAutoSizingKey::Deleted);
 }
 
 bool TextAutoSizingTraits::isDeletedValue(const TextAutoSizingKey& value)
 {
-    return value.style() == TextAutoSizingKey::deletedKeyStyle() && value.doc() == TextAutoSizingKey::deletedKeyDoc();
+    return value.isDeleted();
 }
 #endif
 
@@ -5291,11 +5291,11 @@ HTMLCanvasElement* Document::getCSSCanvasElement(const String& name)
 #if ENABLE(IOS_TEXT_AUTOSIZING)
 void Document::addAutoSizingNode(Node* node, float candidateSize)
 {
-    TextAutoSizingKey key(&node->renderer()->style(), &document());
-    TextAutoSizingMap::AddResult result = m_textAutoSizedNodes.add(key, nullptr);
-    if (result.isNewEntry)
-        result.iterator->value = TextAutoSizingValue::create();
-    result.iterator->value->addNode(node, candidateSize);
+    TextAutoSizingKey key(&node->renderer()->style());
+    auto addResult = m_textAutoSizedNodes.ensure(WTFMove(key), [] {
+        return TextAutoSizingValue::create();
+    });
+    addResult.iterator->value->addNode(node, candidateSize);
 }
 
 void Document::validateAutoSizingNodes()
@@ -5305,23 +5305,17 @@ void Document::validateAutoSizingNodes()
         TextAutoSizingValue* value = keyValuePair.value.get();
         // Update all the nodes in the collection to reflect the new
         // candidate size.
-        if (!value)
-            continue;
-
         value->adjustNodeSizes();
-        if (!value->numNodes())
-            nodesForRemoval.append(keyValuePair.key);
     }
-    for (auto& key : nodesForRemoval)
-        m_textAutoSizedNodes.remove(key);
+    m_textAutoSizedNodes.removeIf([] (TextAutoSizingMap::KeyValuePairType& keyAndValue) {
+        return !keyAndValue.value->numNodes();
+    });
 }
 
 void Document::resetAutoSizingNodes()
 {
-    for (auto& value : m_textAutoSizedNodes.values()) {
-        if (value)
-            value->reset();
-    }
+    for (auto& value : m_textAutoSizedNodes.values())
+        value->reset();
     m_textAutoSizedNodes.clear();
 }
 

Source/WebCore/rendering/TextAutoSizing.cpp

@@ -36,60 +36,24 @@
 
 namespace WebCore {
 
-static PassRefPtr<RenderStyle> cloneRenderStyleWithState(const RenderStyle& currentStyle)
+static Ref<RenderStyle> cloneRenderStyleWithState(const RenderStyle& currentStyle)
 {
-    RefPtr<RenderStyle> newStyle = RenderStyle::clone(&currentStyle);
+    auto newStyle = RenderStyle::clone(&currentStyle);
     if (currentStyle.lastChildState())
         newStyle->setLastChildState();
     if (currentStyle.firstChildState())
         newStyle->setFirstChildState();
-    return newStyle.release();
+    return newStyle;
 }
 
-TextAutoSizingKey::TextAutoSizingKey()
-    : m_style(0)
-    , m_doc(0)
+TextAutoSizingKey::TextAutoSizingKey(DeletedTag)
+    : m_isDeleted(true)
 {
 }
 
-TextAutoSizingKey::TextAutoSizingKey(RenderStyle* style, Document* doc)
-    : m_style(style)
-    , m_doc(doc)
+TextAutoSizingKey::TextAutoSizingKey(RenderStyle* style)
+    : m_style(RenderStyle::clone(style))
 {
-    ref();
-}
-
-TextAutoSizingKey::TextAutoSizingKey(const TextAutoSizingKey& other)
-    : m_style(other.m_style)
-    , m_doc(other.m_doc)
-{
-    ref();
-}
-
-TextAutoSizingKey::~TextAutoSizingKey()
-{
-    deref();
-}
-
-TextAutoSizingKey& TextAutoSizingKey::operator=(const TextAutoSizingKey& other)
-{
-    other.ref();
-    deref();
-    m_style = other.m_style;
-    m_doc = other.m_doc;
-    return *this;
-}
-
-void TextAutoSizingKey::ref() const
-{
-    if (isValidStyle())
-        m_style->ref();
-}
-
-void TextAutoSizingKey::deref() const
-{
-    if (isValidStyle() && isValidDoc())
-        m_style->deref();
 }
 
 int TextAutoSizingValue::numNodes() const
@@ -114,9 +78,7 @@ bool TextAutoSizingValue::adjustNodeSizes()
     // also need to remove the style from the documents m_textAutoSizedNodes
     // collection.  Return true indicates we need to do that removal.
     Vector<RefPtr<Node> > nodesForRemoval;
-    HashSet<RefPtr<Node> >::iterator end = m_autoSizedNodes.end();
-    for (HashSet<RefPtr<Node> >::iterator i = m_autoSizedNodes.begin(); i != end; ++i) {
-        RefPtr<Node> autoSizingNode = *i;
+    for (auto& autoSizingNode : m_autoSizedNodes) {
         RenderText* text = static_cast<RenderText*>(autoSizingNode->renderer());
         if (!text || !text->style().textSizeAdjust().isAuto() || !text->candidateComputedTextSize()) {
             // remove node.
@@ -125,9 +87,8 @@ bool TextAutoSizingValue::adjustNodeSizes()
         }
     }
 
-    unsigned count = nodesForRemoval.size();
-    for (unsigned i = 0; i < count; i++)
-        m_autoSizedNodes.remove(nodesForRemoval[i]);
+    for (auto& node : nodesForRemoval)
+        m_autoSizedNodes.remove(node);
 
     // If we only have one piece of text with the style on the page don't
     // adjust it's size.
@@ -136,9 +97,7 @@ bool TextAutoSizingValue::adjustNodeSizes()
 
     // Compute average size
     float cumulativeSize = 0;
-    end = m_autoSizedNodes.end();
-    for (HashSet<RefPtr<Node> >::iterator i = m_autoSizedNodes.begin(); i != end; ++i) {
-        RefPtr<Node> autoSizingNode = *i;
+    for (auto& autoSizingNode : m_autoSizedNodes) {
         RenderText* renderText = static_cast<RenderText*>(autoSizingNode->renderer());
         cumulativeSize += renderText->candidateComputedTextSize();
     }
@@ -147,9 +106,7 @@ bool TextAutoSizingValue::adjustNodeSizes()
 
     // Adjust sizes
     bool firstPass = true;
-    end = m_autoSizedNodes.end();
-    for (HashSet<RefPtr<Node> >::iterator i = m_autoSizedNodes.begin(); i != end; ++i) {
-        const RefPtr<Node>& autoSizingNode = *i;
+    for (auto& autoSizingNode : m_autoSizedNodes) {
         RenderText* text = static_cast<RenderText*>(autoSizingNode->renderer());
         if (text && text->style().fontDescription().computedSize() != averageSize) {
             float specifiedSize = text->style().fontDescription().specifiedSize();
@@ -160,12 +117,12 @@ bool TextAutoSizingValue::adjustNodeSizes()
                 scaleChange = averageSize / specifiedSize;
             }
 
-            RefPtr<RenderStyle> style = cloneRenderStyleWithState(text->style());
+            auto style = cloneRenderStyleWithState(text->style());
             auto fontDescription = style->fontDescription();
             fontDescription.setComputedSize(averageSize);
             style->setFontDescription(fontDescription);
             style->fontCascade().update(&autoSizingNode->document().fontSelector());
-            text->parent()->setStyle(style.releaseNonNull());
+            text->parent()->setStyle(WTFMove(style));
 
             RenderElement* parentRenderer = text->parent();
             if (parentRenderer->isAnonymousBlock())
@@ -174,10 +131,10 @@ bool TextAutoSizingValue::adjustNodeSizes()
             // If we have a list we should resize ListMarkers separately.
             RenderObject* listMarkerRenderer = parentRenderer->firstChild();
             if (listMarkerRenderer->isListMarker()) {
-                RefPtr<RenderStyle> style = cloneRenderStyleWithState(listMarkerRenderer->style());
+                auto style = cloneRenderStyleWithState(listMarkerRenderer->style());
                 style->setFontDescription(fontDescription);
                 style->fontCascade().update(&autoSizingNode->document().fontSelector());
-                downcast<RenderListMarker>(*listMarkerRenderer).setStyle(style.releaseNonNull());
+                downcast<RenderListMarker>(*listMarkerRenderer).setStyle(WTFMove(style));
             }
 
             // Resize the line height of the parent.
@@ -192,12 +149,12 @@ bool TextAutoSizingValue::adjustNodeSizes()
 
             int lineHeight = specifiedLineHeight * scaleChange;
             if (!lineHeightLength.isFixed() || lineHeightLength.value() != lineHeight) {
-                RefPtr<RenderStyle> newParentStyle = cloneRenderStyleWithState(parentStyle);
+                auto newParentStyle = cloneRenderStyleWithState(parentStyle);
                 newParentStyle->setLineHeight(Length(lineHeight, Fixed));
                 newParentStyle->setSpecifiedLineHeight(lineHeightLength);
                 newParentStyle->setFontDescription(fontDescription);
                 newParentStyle->fontCascade().update(&autoSizingNode->document().fontSelector());
-                parentRenderer->setStyle(newParentStyle.releaseNonNull());
+                parentRenderer->setStyle(WTFMove(newParentStyle));
             }
         }
     }
@@ -207,9 +164,7 @@ bool TextAutoSizingValue::adjustNodeSizes()
 
 void TextAutoSizingValue::reset()
 {
-    HashSet<RefPtr<Node> >::iterator end = m_autoSizedNodes.end();
-    for (HashSet<RefPtr<Node> >::iterator i = m_autoSizedNodes.begin(); i != end; ++i) {
-        const RefPtr<Node>& autoSizingNode = *i;
+    for (auto& autoSizingNode : m_autoSizedNodes) {
         RenderText* text = static_cast<RenderText*>(autoSizingNode->renderer());
         if (!text)
             continue;
@@ -218,10 +173,10 @@ void TextAutoSizingValue::reset()
         float originalSize = fontDescription.specifiedSize();
         if (fontDescription.computedSize() != originalSize) {
             fontDescription.setComputedSize(originalSize);
-            RefPtr<RenderStyle> style = cloneRenderStyleWithState(text->style());
+            auto style = cloneRenderStyleWithState(text->style());
             style->setFontDescription(fontDescription);
             style->fontCascade().update(&autoSizingNode->document().fontSelector());
-            text->parent()->setStyle(style.releaseNonNull());
+            text->parent()->setStyle(WTFMove(style));
         }
         // Reset the line height of the parent.
         RenderElement* parentRenderer = text->parent();
@@ -234,11 +189,11 @@ void TextAutoSizingValue::reset()
         const RenderStyle& parentStyle = parentRenderer->style();
         Length originalLineHeight = parentStyle.specifiedLineHeight();
         if (originalLineHeight != parentStyle.lineHeight()) {
-            RefPtr<RenderStyle> newParentStyle = cloneRenderStyleWithState(parentStyle);
+            auto newParentStyle = cloneRenderStyleWithState(parentStyle);
             newParentStyle->setLineHeight(originalLineHeight);
             newParentStyle->setFontDescription(fontDescription);
             newParentStyle->fontCascade().update(&autoSizingNode->document().fontSelector());
-            parentRenderer->setStyle(newParentStyle.releaseNonNull());
+            parentRenderer->setStyle(WTFMove(newParentStyle));
         }
     }
 }

Source/WebCore/rendering/TextAutoSizing.h

@@ -40,29 +40,29 @@ class Node;
 
 class TextAutoSizingKey {
 public:
-    TextAutoSizingKey();
-    TextAutoSizingKey(RenderStyle*, Document*);
-    ~TextAutoSizingKey();
-    TextAutoSizingKey(const TextAutoSizingKey&);
-    TextAutoSizingKey& operator=(const TextAutoSizingKey&);
-    Document* doc() const { return m_doc; }
-    RenderStyle* style() const { return m_style; }
-    inline bool isValidDoc() const { return m_doc && m_doc != deletedKeyDoc(); }
-    inline bool isValidStyle() const { return m_style && m_style != deletedKeyStyle(); }
-    static Document* deletedKeyDoc() { return reinterpret_cast<Document*>(-1); }
-    static RenderStyle* deletedKeyStyle() { return reinterpret_cast<RenderStyle*>(-1); }
+    TextAutoSizingKey() = default;
+    enum DeletedTag { Deleted };
+    explicit TextAutoSizingKey(DeletedTag);
+    explicit TextAutoSizingKey(RenderStyle*);
+    TextAutoSizingKey(TextAutoSizingKey&&) = default;
+
+    TextAutoSizingKey& operator=(TextAutoSizingKey&&) = default;
+
+    RenderStyle* style() const { return m_style.get(); }
+    inline bool isDeleted() const { return m_isDeleted; }
+
 private:
-    void ref() const;
-    void deref() const;
-    RenderStyle* m_style;
-    Document* m_doc;
+    RefPtr<RenderStyle> m_style;
+    bool m_isDeleted { false };
 };
 
 inline bool operator==(const TextAutoSizingKey& a, const TextAutoSizingKey& b)
 {
-    if (a.isValidStyle() && b.isValidStyle())
-        return a.style()->equalForTextAutosizing(b.style());
-    return a.style() == b.style();
+    if (a.isDeleted() || b.isDeleted())
+        return false;
+    if (!a.style() || !b.style())
+        return a.style() == b.style();
+    return a.style()->equalForTextAutosizing(b.style());
 }
 
 struct TextAutoSizingHash {