Oilpan: move editing objects to the heap.

Source/core/editing/markup.cpp

Index: Source/core/editing/markup.cpp
diff --git a/Source/core/editing/markup.cpp b/Source/core/editing/markup.cpp
index d6f21373d46312daa8e3bb0b3c17e7f4833c6fda..d0547298e7a5df48f96e67ed7c64218612e4aa70 100644
--- a/Source/core/editing/markup.cpp
+++ b/Source/core/editing/markup.cpp
@@ -68,14 +68,14 @@ using namespace HTMLNames;
 
 static bool propertyMissingOrEqualToNone(StylePropertySet*, CSSPropertyID);
 
-class AttributeChange {
+class AttributeChange : public NoBaseWillBeGarbageCollectedFinalized<AttributeChange> {
 public:
-    AttributeChange()
-        : m_name(nullAtom, nullAtom, nullAtom)
+    static PassOwnPtrWillBeRawPtr<AttributeChange> create(PassRefPtrWillBeRawPtr<Element> element, const QualifiedName& name, const String& value)
     {
+        return adoptPtrWillBeNoop(new AttributeChange(element, name, value));
     }
 
-    AttributeChange(PassRefPtr<Element> element, const QualifiedName& name, const String& value)
+    AttributeChange(PassRefPtrWillBeRawPtr<Element> element, const QualifiedName& name, const String& value)
         : m_element(element), m_name(name), m_value(value)
     {
     }
@@ -85,15 +85,20 @@ public:
         m_element->setAttribute(m_name, AtomicString(m_value));
     }
 
+    void trace(Visitor* visitor)
+    {
+        visitor->trace(m_element);
+    }
+
 private:
-    RefPtr<Element> m_element;
+    RefPtrWillBeMember<Element> m_element;
     QualifiedName m_name;
     String m_value;
 };
 
 static void completeURLs(DocumentFragment& fragment, const String& baseURL)
 {
-    Vector<AttributeChange> changes;
+    WillBeHeapVector<OwnPtrWillBeMember<AttributeChange> > changes;
 
     KURL parsedBaseURL(ParsedURLString, baseURL);
 
@@ -104,20 +109,20 @@ static void completeURLs(DocumentFragment& fragment, const String& baseURL)
         for (unsigned i = 0; i < length; i++) {
             const Attribute& attribute = element->attributeItem(i);
             if (element->isURLAttribute(attribute) && !attribute.value().isEmpty())
-                changes.append(AttributeChange(element, attribute.name(), KURL(parsedBaseURL, attribute.value()).string()));
+                changes.append(AttributeChange::create(element, attribute.name(), KURL(parsedBaseURL, attribute.value()).string()));
         }
     }
 
     size_t numChanges = changes.size();
     for (size_t i = 0; i < numChanges; ++i)
-        changes[i].apply();
+        changes[i]->apply();
 }
 
 class StyledMarkupAccumulator FINAL : public MarkupAccumulator {
 public:
     enum RangeFullySelectsNode { DoesFullySelectNode, DoesNotFullySelectNode };
 
-    StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs, EAnnotateForInterchange, const Range*, Node* highestNodeToBeSerialized = 0);
+    StyledMarkupAccumulator(WillBeHeapVector<RawPtrWillBeMember<Node> >* nodes, EAbsoluteURLs, EAnnotateForInterchange, RawPtr<const Range>, Node* highestNodeToBeSerialized = 0);
     Node* serializeNodes(Node* startNode, Node* pastEnd);
     void appendString(const String& s) { return MarkupAccumulator::appendString(s); }
     void wrapWithNode(Node&, bool convertBlocksToInlines = false, RangeFullySelectsNode = DoesFullySelectNode);
@@ -145,12 +150,11 @@ private:
 
     Vector<String> m_reversedPrecedingMarkup;
     const EAnnotateForInterchange m_shouldAnnotate;
-    Node* m_highestNodeToBeSerialized;
-    RefPtr<EditingStyle> m_wrappingStyle;
+    RawPtrWillBeMember<Node> m_highestNodeToBeSerialized;
+    RefPtrWillBeMember<EditingStyle> m_wrappingStyle;

[haraken, 2014/05/28 15:35:25]

Who traces these Members?

Ian: It'd be great if the plugin could catch this :)

[haraken, 2014/05/29 01:14:15]

Ignore this comment :) I found that this object is stack-allocated.

 };
 
-inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate,
-    const Range* range, Node* highestNodeToBeSerialized)
+inline StyledMarkupAccumulator::StyledMarkupAccumulator(WillBeHeapVector<RawPtrWillBeMember<Node> >* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate, RawPtr<const Range> range, Node* highestNodeToBeSerialized)
     : MarkupAccumulator(nodes, shouldResolveURLs, range)
     , m_shouldAnnotate(shouldAnnotate)
     , m_highestNodeToBeSerialized(highestNodeToBeSerialized)
@@ -216,7 +220,7 @@ void StyledMarkupAccumulator::appendText(StringBuilder& out, Text& text)
     const bool parentIsTextarea = text.parentElement() && text.parentElement()->tagQName() == textareaTag;
     const bool wrappingSpan = shouldApplyWrappingStyle(text) && !parentIsTextarea;
     if (wrappingSpan) {
-        RefPtr<EditingStyle> wrappingStyle = m_wrappingStyle->copy();
+        RefPtrWillBeRawPtr<EditingStyle> wrappingStyle = m_wrappingStyle->copy();
         // FIXME: <rdar://problem/5371536> Style rules that match pasted content can change it's appearance
         // Make sure spans are inline style in paste side e.g. span { display: block }.
         wrappingStyle->forceInline();
@@ -289,7 +293,7 @@ void StyledMarkupAccumulator::appendElement(StringBuilder& out, Element& element
     }
 
     if (shouldOverrideStyleAttr) {
-        RefPtr<EditingStyle> newInlineStyle;
+        RefPtrWillBeRawPtr<EditingStyle> newInlineStyle;

[haraken, 2014/05/28 15:35:25]

= nullptr;

Mads prefers always-initialization even if it compiles.

[Mads Ager (chromium), 2014/05/28 16:09:44]

I do prefer that. :-)

On the other hand I don't have much of a problem with not doing it if we are
sure that compilers catch this. When I changed RawPtr to not zero-initialize
there were bugs that were not caught by clang where we were missing nullptr
initialization of these. If we are sure that compilers will catch uninitialized
RawPtrs I'm fine with leaving it out as well. 

[sof, 2014/05/28 22:06:06]

Complied.

 
         if (shouldApplyWrappingStyle(element)) {
             newInlineStyle = m_wrappingStyle->copy();
@@ -340,7 +344,7 @@ Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
 Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, Node* pastEnd, NodeTraversalMode traversalMode)
 {
     const bool shouldEmit = traversalMode == EmitString;
-    Vector<Node*> ancestorsToClose;
+    WillBeHeapVector<RawPtrWillBeMember<Node> > ancestorsToClose;
     Node* next;
     Node* lastClosed = 0;
     for (Node* n = startNode; n != pastEnd; n = next) {
@@ -475,7 +479,7 @@ static bool needInterchangeNewlineAfter(const VisiblePosition& v)
     return isEndOfParagraph(v) && isStartOfParagraph(next) && !(isHTMLBRElement(*upstreamNode) && upstreamNode == downstreamNode);
 }
 
-static PassRefPtr<EditingStyle> styleFromMatchedRulesAndInlineDecl(const Node* node)
+static PassRefPtrWillBeRawPtr<EditingStyle> styleFromMatchedRulesAndInlineDecl(const Node* node)
 {
     if (!node->isHTMLElement())
         return nullptr;
@@ -483,7 +487,7 @@ static PassRefPtr<EditingStyle> styleFromMatchedRulesAndInlineDecl(const Node* n
     // FIXME: Having to const_cast here is ugly, but it is quite a bit of work to untangle
     // the non-const-ness of styleFromMatchedRulesForElement.
     HTMLElement* element = const_cast<HTMLElement*>(toHTMLElement(node));
-    RefPtr<EditingStyle> style = EditingStyle::create(element->inlineStyle());
+    RefPtrWillBeRawPtr<EditingStyle> style = EditingStyle::create(element->inlineStyle());
     style->mergeStyleFromRules(element);
     return style.release();
 }
@@ -541,7 +545,7 @@ static Node* highestAncestorToWrapMarkup(const Range* range, EAnnotateForInterch
 
 // FIXME: Shouldn't we omit style info when annotate == DoNotAnnotateForInterchange?
 // FIXME: At least, annotation and style info should probably not be included in range.markupString()
-static String createMarkupInternal(Document& document, const Range* range, const Range* updatedRange, Vector<Node*>* nodes,
+static String createMarkupInternal(Document& document, const Range* range, const Range* updatedRange, WillBeHeapVector<RawPtrWillBeMember<Node> >* nodes,
     EAnnotateForInterchange shouldAnnotate, bool convertBlocksToInlines, EAbsoluteURLs shouldResolveURLs, Node* constrainingAncestor)
 {
     ASSERT(range);
@@ -586,7 +590,7 @@ static String createMarkupInternal(Document& document, const Range* range, const
         // Also include all of the ancestors of lastClosed up to this special ancestor.
         for (ContainerNode* ancestor = lastClosed->parentNode(); ancestor; ancestor = ancestor->parentNode()) {
             if (ancestor == fullySelectedRoot && !convertBlocksToInlines) {
-                RefPtr<EditingStyle> fullySelectedRootStyle = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
+                RefPtrWillBeRawPtr<EditingStyle> fullySelectedRootStyle = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
 
                 // Bring the background attribute over, but not as an attribute because a background attribute on a div
                 // appears to have no effect.
@@ -624,7 +628,7 @@ static String createMarkupInternal(Document& document, const Range* range, const
     return accumulator.takeResults();
 }
 
-String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterchange shouldAnnotate, bool convertBlocksToInlines, EAbsoluteURLs shouldResolveURLs, Node* constrainingAncestor)
+String createMarkup(const Range* range, WillBeHeapVector<RawPtrWillBeMember<Node> >* nodes, EAnnotateForInterchange shouldAnnotate, bool convertBlocksToInlines, EAbsoluteURLs shouldResolveURLs, Node* constrainingAncestor)
 {
     if (!range)
         return emptyString();
@@ -734,7 +738,7 @@ PassRefPtrWillBeRawPtr<DocumentFragment> createFragmentFromMarkupWithContext(Doc
     return fragment;
 }
 
-String createMarkup(const Node* node, EChildrenOnly childrenOnly, Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, Vector<QualifiedName>* tagNamesToSkip)
+String createMarkup(const Node* node, EChildrenOnly childrenOnly, WillBeHeapVector<RawPtrWillBeMember<Node> >* nodes, EAbsoluteURLs shouldResolveURLs, Vector<QualifiedName>* tagNamesToSkip)
 {
     if (!node)
         return "";
@@ -832,7 +836,7 @@ PassRefPtrWillBeRawPtr<DocumentFragment> createFragmentFromText(Range* context,
     if (shouldPreserveNewline(*context)) {
         fragment->appendChild(document.createTextNode(string));
         if (string.endsWith('\n')) {
-            RefPtr<Element> element = createBreakElement(document);
+            RefPtrWillBeRawPtr<Element> element = createBreakElement(document);
             element->setAttribute(classAttr, AppleInterchangeNewline);
             fragment->appendChild(element.release());
         }
@@ -861,7 +865,7 @@ PassRefPtrWillBeRawPtr<DocumentFragment> createFragmentFromText(Range* context,
     for (size_t i = 0; i < numLines; ++i) {
         const String& s = list[i];
 
-        RefPtr<Element> element;
+        RefPtrWillBeRawPtr<Element> element = nullptr;
         if (s.isEmpty() && i + 1 == numLines) {
             // For last line, use the "magic BR" rather than a P.
             element = createBreakElement(document);