Bug 1154701 part 6 - Clean up nsHTMLEditor::SetInlinePropertyOnNodeImpl; r=ehsan
☠☠ backed out by 329dd852c06b ☠ ☠
authorAryeh Gregor <ayg@aryeh.name>
Wed, 22 Apr 2015 14:27:17 +0300
changeset 240522 a4d41b284d927f11d039a4be54a61630d9706560
parent 240521 1ab63df2b8eb262ef43b5ce226a6e5287541c1e5
child 240523 8bd58656d297655e4980b353a42d3d7f4331b7a4
push id28636
push userkwierso@gmail.com
push dateThu, 23 Apr 2015 00:16:12 +0000
treeherdermozilla-central@a5af73b32ac8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1154701
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1154701 part 6 - Clean up nsHTMLEditor::SetInlinePropertyOnNodeImpl; r=ehsan
editor/libeditor/nsHTMLEditor.h
editor/libeditor/nsHTMLEditorStyle.cpp
--- a/editor/libeditor/nsHTMLEditor.h
+++ b/editor/libeditor/nsHTMLEditor.h
@@ -955,20 +955,20 @@ friend class nsWSRunObject;
 friend class nsHTMLEditorEventListener;
 
 private:
   // Helpers
   bool IsSimpleModifiableNode(nsIContent* aContent,
                               nsIAtom* aProperty,
                               const nsAString* aAttribute,
                               const nsAString* aValue);
-  nsresult SetInlinePropertyOnNodeImpl(nsIContent* aNode,
-                                       nsIAtom* aProperty,
+  nsresult SetInlinePropertyOnNodeImpl(nsIContent& aNode,
+                                       nsIAtom& aProperty,
                                        const nsAString* aAttribute,
-                                       const nsAString* aValue);
+                                       const nsAString& aValue);
   typedef enum { eInserted, eAppended } InsertedOrAppended;
   void DoContentInserted(nsIDocument* aDocument, nsIContent* aContainer,
                          nsIContent* aChild, int32_t aIndexInContainer,
                          InsertedOrAppended aInsertedOrAppended);
   already_AddRefed<mozilla::dom::Element> GetElementOrParentByTagName(
       const nsAString& aTagName, nsINode* aNode);
   already_AddRefed<mozilla::dom::Element> CreateElementWithDefaults(
       const nsAString& aTagName);
--- a/editor/libeditor/nsHTMLEditorStyle.cpp
+++ b/editor/libeditor/nsHTMLEditorStyle.cpp
@@ -376,118 +376,114 @@ nsHTMLEditor::SetInlinePropertyOnTextNod
   }
 
   // Reparent the node inside inline node with appropriate {attribute,value}
   return SetInlinePropertyOnNode(text, &aProperty, aAttribute, &aValue);
 }
 
 
 nsresult
-nsHTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent* aNode,
-                                          nsIAtom* aProperty,
+nsHTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent& aNode,
+                                          nsIAtom& aProperty,
                                           const nsAString* aAttribute,
-                                          const nsAString* aValue)
+                                          const nsAString& aValue)
 {
-  MOZ_ASSERT(aNode && aProperty);
-  MOZ_ASSERT(aValue);
-
   nsCOMPtr<nsIAtom> attrAtom = aAttribute ? do_GetAtom(*aAttribute) : nullptr;
 
   // If this is an element that can't be contained in a span, we have to
   // recurse to its children.
-  if (!TagCanContain(*nsGkAtoms::span, *aNode)) {
-    if (aNode->HasChildren()) {
-      nsCOMArray<nsIContent> arrayOfNodes;
+  if (!TagCanContain(*nsGkAtoms::span, aNode)) {
+    if (aNode.HasChildren()) {
+      nsTArray<OwningNonNull<nsIContent>> arrayOfNodes;
 
       // Populate the list.
-      for (nsIContent* child = aNode->GetFirstChild();
+      for (nsCOMPtr<nsIContent> child = aNode.GetFirstChild();
            child;
            child = child->GetNextSibling()) {
         if (IsEditable(child) && !IsEmptyTextNode(this, child)) {
-          arrayOfNodes.AppendObject(child);
+          arrayOfNodes.AppendElement(*child);
         }
       }
 
       // Then loop through the list, set the property on each node.
-      int32_t listCount = arrayOfNodes.Count();
-      for (int32_t j = 0; j < listCount; ++j) {
-        nsresult rv = SetInlinePropertyOnNode(arrayOfNodes[j], aProperty,
-                                              aAttribute, aValue);
+      for (auto& node : arrayOfNodes) {
+        nsresult rv = SetInlinePropertyOnNode(node, &aProperty, aAttribute,
+                                              &aValue);
         NS_ENSURE_SUCCESS(rv, rv);
       }
     }
     return NS_OK;
   }
 
   // First check if there's an adjacent sibling we can put our node into.
   nsresult res;
-  nsCOMPtr<nsIContent> previousSibling = GetPriorHTMLSibling(aNode);
-  nsCOMPtr<nsIContent> nextSibling = GetNextHTMLSibling(aNode);
-  if (IsSimpleModifiableNode(previousSibling, aProperty, aAttribute, aValue)) {
-    res = MoveNode(aNode, previousSibling, -1);
+  nsCOMPtr<nsIContent> previousSibling = GetPriorHTMLSibling(&aNode);
+  nsCOMPtr<nsIContent> nextSibling = GetNextHTMLSibling(&aNode);
+  if (IsSimpleModifiableNode(previousSibling, &aProperty, aAttribute, &aValue)) {
+    res = MoveNode(&aNode, previousSibling, -1);
     NS_ENSURE_SUCCESS(res, res);
-    if (IsSimpleModifiableNode(nextSibling, aProperty, aAttribute, aValue)) {
+    if (IsSimpleModifiableNode(nextSibling, &aProperty, aAttribute, &aValue)) {
       res = JoinNodes(*previousSibling, *nextSibling);
       NS_ENSURE_SUCCESS(res, res);
     }
     return NS_OK;
   }
-  if (IsSimpleModifiableNode(nextSibling, aProperty, aAttribute, aValue)) {
-    res = MoveNode(aNode, nextSibling, 0);
+  if (IsSimpleModifiableNode(nextSibling, &aProperty, aAttribute, &aValue)) {
+    res = MoveNode(&aNode, nextSibling, 0);
     NS_ENSURE_SUCCESS(res, res);
     return NS_OK;
   }
 
-  // don't need to do anything if property already set on node
-  if (mHTMLCSSUtils->IsCSSEditableProperty(aNode, aProperty, aAttribute)) {
+  // Don't need to do anything if property already set on node
+  if (mHTMLCSSUtils->IsCSSEditableProperty(&aNode, &aProperty, aAttribute)) {
     if (mHTMLCSSUtils->IsCSSEquivalentToHTMLInlineStyleSet(
-          aNode, aProperty, aAttribute, *aValue, nsHTMLCSSUtils::eComputed)) {
+          &aNode, &aProperty, aAttribute, aValue, nsHTMLCSSUtils::eComputed)) {
       return NS_OK;
     }
-  } else if (IsTextPropertySetByContent(aNode, aProperty,
-                                        aAttribute, aValue)) {
+  } else if (IsTextPropertySetByContent(&aNode, &aProperty,
+                                        aAttribute, &aValue)) {
     return NS_OK;
   }
 
   bool useCSS = (IsCSSEnabled() &&
-                 mHTMLCSSUtils->IsCSSEditableProperty(aNode, aProperty, aAttribute)) ||
+                 mHTMLCSSUtils->IsCSSEditableProperty(&aNode, &aProperty, aAttribute)) ||
                 // bgcolor is always done using CSS
                 aAttribute->EqualsLiteral("bgcolor");
 
   if (useCSS) {
     nsCOMPtr<dom::Element> tmp;
     // We only add style="" to <span>s with no attributes (bug 746515).  If we
     // don't have one, we need to make one.
-    if (aNode->IsHTMLElement(nsGkAtoms::span) &&
-        !aNode->AsElement()->GetAttrCount()) {
-      tmp = aNode->AsElement();
+    if (aNode.IsHTMLElement(nsGkAtoms::span) &&
+        !aNode.AsElement()->GetAttrCount()) {
+      tmp = aNode.AsElement();
     } else {
-      tmp = InsertContainerAbove(aNode, nsGkAtoms::span);
+      tmp = InsertContainerAbove(&aNode, nsGkAtoms::span);
       NS_ENSURE_STATE(tmp);
     }
 
     // Add the CSS styles corresponding to the HTML style request
     int32_t count;
     res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(tmp->AsDOMNode(),
-                                                     aProperty, aAttribute,
-                                                     aValue, &count, false);
+                                                     &aProperty, aAttribute,
+                                                     &aValue, &count, false);
     NS_ENSURE_SUCCESS(res, res);
     return NS_OK;
   }
 
   // is it already the right kind of node, but with wrong attribute?
-  if (aNode->IsHTMLElement(aProperty)) {
+  if (aNode.IsHTMLElement(&aProperty)) {
     // Just set the attribute on it.
-    nsCOMPtr<nsIDOMElement> elem = do_QueryInterface(aNode);
-    return SetAttribute(elem, *aAttribute, *aValue);
+    nsCOMPtr<nsIDOMElement> elem = do_QueryInterface(&aNode);
+    return SetAttribute(elem, *aAttribute, aValue);
   }
 
   // ok, chuck it in its very own container
-  nsCOMPtr<Element> tmp = InsertContainerAbove(aNode, aProperty, attrAtom,
-                                               aValue);
+  nsCOMPtr<Element> tmp = InsertContainerAbove(&aNode, &aProperty, attrAtom,
+                                               &aValue);
   NS_ENSURE_STATE(tmp);
 
   return NS_OK;
 }
 
 
 nsresult
 nsHTMLEditor::SetInlinePropertyOnNode(nsIDOMNode *aNode,
@@ -522,18 +518,18 @@ nsHTMLEditor::SetInlinePropertyOnNode(ns
   nsCOMPtr<nsINode> parent = aNode->GetParentNode();
   NS_ENSURE_STATE(parent);
 
   nsresult res = RemoveStyleInside(aNode->AsDOMNode(), aProperty, aAttribute);
   NS_ENSURE_SUCCESS(res, res);
 
   if (aNode->GetParentNode()) {
     // The node is still where it was
-    return SetInlinePropertyOnNodeImpl(aNode, aProperty,
-                                       aAttribute, aValue);
+    return SetInlinePropertyOnNodeImpl(*aNode, *aProperty,
+                                       aAttribute, *aValue);
   }
 
   // It's vanished.  Use the old siblings for reference to construct a
   // list.  But first, verify that the previous/next siblings are still
   // where we expect them; otherwise we have to give up.
   if ((previousSibling && previousSibling->GetParentNode() != parent) ||
       (nextSibling && nextSibling->GetParentNode() != parent)) {
     return NS_ERROR_UNEXPECTED;
@@ -545,18 +541,18 @@ nsHTMLEditor::SetInlinePropertyOnNode(ns
     if (IsEditable(cur)) {
       nodesToSet.AppendObject(cur);
     }
     cur = cur->GetNextSibling();
   }
 
   int32_t nodesToSetCount = nodesToSet.Count();
   for (int32_t k = 0; k < nodesToSetCount; k++) {
-    res = SetInlinePropertyOnNodeImpl(nodesToSet[k], aProperty,
-                                      aAttribute, aValue);
+    res = SetInlinePropertyOnNodeImpl(*nodesToSet[k], *aProperty,
+                                      aAttribute, *aValue);
     NS_ENSURE_SUCCESS(res, res);
   }
 
   return NS_OK;
 }
 
 
 nsresult