Bug 1460509 - part 73: Make HTMLEditRules::WillMakeList() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 17 May 2018 20:51:56 +0900
changeset 798791 e98e1f80a80fdfcfd57d56579a8589285c9a5df1
parent 798790 84c52ed3b7c0c73944a053dcbe72e89fca525c6b
child 798792 1aacc148d12799d83bc74b115da1e2377f063c69
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 73: Make HTMLEditRules::WillMakeList() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato MozReview-Commit-ID: 6qTXENBW8tV
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3904,22 +3904,44 @@ HTMLEditRules::WillMakeList(const nsAStr
 
   *aHandled = true;
 
   rv = NormalizeSelection();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
+  // MakeList() creates AutoSelectionRestorer.
+  // Therefore, even if it returns NS_OK, editor might have been destroyed
+  // at restoring Selection.
+  rv = MakeList(listType, aEntireList, aBulletType, aCancel, *itemType);
+  if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED) ||
+      NS_WARN_IF(!CanHandleEditAction())) {
+    return NS_ERROR_EDITOR_DESTROYED;
+  }
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return NS_OK;
+}
+
+nsresult
+HTMLEditRules::MakeList(nsAtom& aListType,
+                        bool aEntireList,
+                        const nsAString* aBulletType,
+                        bool* aCancel,
+                        nsAtom& aItemType)
+{
   AutoSelectionRestorer selectionRestorer(&SelectionRef(), &HTMLEditorRef());
 
   nsTArray<OwningNonNull<nsINode>> arrayOfNodes;
-  rv = GetListActionNodes(arrayOfNodes,
-                          aEntireList ? EntireList::yes : EntireList::no,
-                          TouchContent::yes);
+  nsresult rv =
+    GetListActionNodes(arrayOfNodes,
+                       aEntireList ? EntireList::yes : EntireList::no,
+                       TouchContent::yes);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // check if all our nodes are <br>s, or empty inlines
   bool bOnlyBreaks = true;
   for (auto& curNode : arrayOfNodes) {
     // if curNode is not a Break or empty inline, we're done
@@ -3932,16 +3954,19 @@ HTMLEditRules::WillMakeList(const nsAStr
 
   // if no nodes, we make empty list.  Ditto if the user tried to make a list
   // of some # of breaks.
   if (arrayOfNodes.IsEmpty() || bOnlyBreaks) {
     // if only breaks, delete them
     if (bOnlyBreaks) {
       for (auto& node : arrayOfNodes) {
         rv = HTMLEditorRef().DeleteNodeWithTransaction(*node);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       }
     }
 
     nsRange* firstRange = SelectionRef().GetRangeAt(0);
     if (NS_WARN_IF(!firstRange)) {
@@ -3950,50 +3975,58 @@ HTMLEditRules::WillMakeList(const nsAStr
 
     EditorDOMPoint atStartOfSelection(firstRange->StartRef());
     if (NS_WARN_IF(!atStartOfSelection.IsSet())) {
       return NS_ERROR_FAILURE;
     }
 
     // Make sure we can put a list here.
     if (!HTMLEditorRef().CanContainTag(*atStartOfSelection.GetContainer(),
-                                       listType)) {
+                                       aListType)) {
       *aCancel = true;
       return NS_OK;
     }
 
     SplitNodeResult splitAtSelectionStartResult =
-      MaybeSplitAncestorsForInsertWithTransaction(listType, atStartOfSelection);
+      MaybeSplitAncestorsForInsertWithTransaction(aListType,
+                                                  atStartOfSelection);
     if (NS_WARN_IF(splitAtSelectionStartResult.Failed())) {
       return splitAtSelectionStartResult.Rv();
     }
     RefPtr<Element> theList =
       HTMLEditorRef().CreateNodeWithTransaction(
-                        *listType,
-                        splitAtSelectionStartResult.SplitPoint());
+                        aListType, splitAtSelectionStartResult.SplitPoint());
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(!theList)) {
       return NS_ERROR_FAILURE;
     }
 
     EditorRawDOMPoint atFirstListItemToInsertBefore(theList, 0);
     RefPtr<Element> theListItem =
-      HTMLEditorRef().CreateNodeWithTransaction(*itemType,
+      HTMLEditorRef().CreateNodeWithTransaction(aItemType,
                                                 atFirstListItemToInsertBefore);
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(!theListItem)) {
       return NS_ERROR_FAILURE;
     }
 
     // remember our new block for postprocessing
     mNewBlock = theListItem;
-    // put selection in new list item
-    *aHandled = true;
+    // Put selection in new list item and don't restore the Selection.
+    selectionRestorer.Abort();
     ErrorResult error;
     SelectionRef().Collapse(EditorRawDOMPoint(theListItem, 0), error);
-    // Don't restore the selection
-    selectionRestorer.Abort();
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      error.SuppressException();
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(!error.Failed())) {
       return error.StealNSResult();
     }
     return NS_OK;
   }
 
   // if there is only one node in the array, and it is a list, div, or
   // blockquote, then look inside of it until we find inner list or content.
@@ -4021,16 +4054,19 @@ HTMLEditRules::WillMakeList(const nsAStr
     }
 
     // If curNode is a break, delete it, and quit remembering prev list item.
     // If an empty inline container, delete it, but still remember the previous
     // item.
     if (HTMLEditorRef().IsEditable(curNode) &&
         (TextEditUtils::IsBreak(curNode) || IsEmptyInline(curNode))) {
       rv = HTMLEditorRef().DeleteNodeWithTransaction(*curNode);
+      if (NS_WARN_IF(!CanHandleEditAction())) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       if (TextEditUtils::IsBreak(curNode)) {
         prevListItem = nullptr;
       }
       continue;
     }
@@ -4038,152 +4074,189 @@ HTMLEditRules::WillMakeList(const nsAStr
     if (HTMLEditUtils::IsList(curNode)) {
       // do we have a curList already?
       if (curList && !EditorUtils::IsDescendantOf(*curNode, *curList)) {
         // move all of our children into curList.  cheezy way to do it: move
         // whole list and then RemoveContainerWithTransaction() on the list.
         // ConvertListType first: that routine handles converting the list
         // item types, if needed.
         rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*curNode, *curList);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
         CreateElementResult convertListTypeResult =
-          ConvertListType(*curNode->AsElement(), *listType, *itemType);
+          ConvertListType(*curNode->AsElement(), aListType, aItemType);
         if (NS_WARN_IF(convertListTypeResult.Failed())) {
           return convertListTypeResult.Rv();
         }
         rv = HTMLEditorRef().RemoveBlockContainerWithTransaction(
                                *convertListTypeResult.GetNewNode());
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
         newBlock = convertListTypeResult.forget();
       } else {
         // replace list with new list type
         CreateElementResult convertListTypeResult =
-          ConvertListType(*curNode->AsElement(), *listType, *itemType);
+          ConvertListType(*curNode->AsElement(), aListType, aItemType);
         if (NS_WARN_IF(convertListTypeResult.Failed())) {
           return convertListTypeResult.Rv();
         }
         curList = convertListTypeResult.forget();
       }
       prevListItem = nullptr;
       continue;
     }
 
     EditorRawDOMPoint atCurNode(curNode);
     if (NS_WARN_IF(!atCurNode.IsSet())) {
       return NS_ERROR_FAILURE;
     }
     MOZ_ASSERT(atCurNode.IsSetAndValid());
     if (HTMLEditUtils::IsListItem(curNode)) {
-      if (!atCurNode.IsContainerHTMLElement(listType)) {
+      if (!atCurNode.IsContainerHTMLElement(&aListType)) {
         // list item is in wrong type of list. if we don't have a curList,
         // split the old list and make a new list of correct type.
         if (!curList || EditorUtils::IsDescendantOf(*curNode, *curList)) {
           if (NS_WARN_IF(!atCurNode.GetContainerAsContent())) {
             return NS_ERROR_FAILURE;
           }
           ErrorResult error;
           nsCOMPtr<nsIContent> newLeftNode =
             HTMLEditorRef().SplitNodeWithTransaction(atCurNode, error);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            error.SuppressException();
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(error.Failed())) {
             return error.StealNSResult();
           }
           newBlock = newLeftNode ? newLeftNode->AsElement() : nullptr;
           EditorRawDOMPoint atParentOfCurNode(atCurNode.GetContainer());
           curList =
-            HTMLEditorRef().CreateNodeWithTransaction(*listType,
+            HTMLEditorRef().CreateNodeWithTransaction(aListType,
                                                       atParentOfCurNode);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(!curList)) {
             return NS_ERROR_FAILURE;
           }
         }
         // move list item to new list
         rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*curNode, *curList);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
         // convert list item type if needed
-        if (!curNode->IsHTMLElement(itemType)) {
+        if (!curNode->IsHTMLElement(&aItemType)) {
           newBlock =
             HTMLEditorRef().ReplaceContainerWithTransaction(
-                              *curNode->AsElement(), *itemType);
+                              *curNode->AsElement(), aItemType);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(!newBlock)) {
             return NS_ERROR_FAILURE;
           }
         }
       } else {
         // item is in right type of list.  But we might still have to move it.
         // and we might need to convert list item types.
         if (!curList) {
           curList = atCurNode.GetContainerAsElement();
         } else if (atCurNode.GetContainer() != curList) {
           // move list item to new list
           rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*curNode, *curList);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return rv;
           }
         }
-        if (!curNode->IsHTMLElement(itemType)) {
+        if (!curNode->IsHTMLElement(&aItemType)) {
           newBlock =
             HTMLEditorRef().ReplaceContainerWithTransaction(
-                              *curNode->AsElement(), *itemType);
+                              *curNode->AsElement(), aItemType);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(!newBlock)) {
             return NS_ERROR_FAILURE;
           }
         }
       }
       nsCOMPtr<Element> curElement = do_QueryInterface(curNode);
       if (NS_WARN_IF(!curElement)) {
         return NS_ERROR_FAILURE;
       }
       if (aBulletType && !aBulletType->IsEmpty()) {
         rv = HTMLEditorRef().SetAttributeWithTransaction(*curElement,
                                                          *nsGkAtoms::type,
                                                          *aBulletType);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       } else {
         rv = HTMLEditorRef().RemoveAttributeWithTransaction(*curElement,
                                                             *nsGkAtoms::type);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       }
       continue;
     }
 
     // if we hit a div clear our prevListItem, insert divs contents
     // into our node array, and remove the div
     if (curNode->IsHTMLElement(nsGkAtoms::div)) {
       prevListItem = nullptr;
       int32_t j = i + 1;
       GetInnerContent(*curNode, arrayOfNodes, &j);
       rv = HTMLEditorRef().RemoveContainerWithTransaction(
                              *curNode->AsElement());
+      if (NS_WARN_IF(!CanHandleEditAction())) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       listCount = arrayOfNodes.Length();
       continue;
     }
 
     // need to make a list to put things in if we haven't already,
     if (!curList) {
       SplitNodeResult splitCurNodeResult =
-        MaybeSplitAncestorsForInsertWithTransaction(listType, atCurNode);
+        MaybeSplitAncestorsForInsertWithTransaction(aListType, atCurNode);
       if (NS_WARN_IF(splitCurNodeResult.Failed())) {
         return splitCurNodeResult.Rv();
       }
       curList =
         HTMLEditorRef().CreateNodeWithTransaction(
-                          *listType, splitCurNodeResult.SplitPoint());
+                          aListType, splitCurNodeResult.SplitPoint());
+      if (NS_WARN_IF(!CanHandleEditAction())) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
       if (NS_WARN_IF(!curList)) {
         return NS_ERROR_FAILURE;
       }
       // remember our new block for postprocessing
       mNewBlock = curList;
       // curList is now the correct thing to put curNode in
       prevListItem = nullptr;
 
@@ -4195,31 +4268,40 @@ HTMLEditRules::WillMakeList(const nsAStr
     // if curNode isn't a list item, we must wrap it in one
     nsCOMPtr<Element> listItem;
     if (!HTMLEditUtils::IsListItem(curNode)) {
       if (IsInlineNode(curNode) && prevListItem) {
         // this is a continuation of some inline nodes that belong together in
         // the same list item.  use prevListItem
         rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*curNode,
                                                           *prevListItem);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return NS_ERROR_EDITOR_DESTROYED;
+        }
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
       } else {
         // don't wrap li around a paragraph.  instead replace paragraph with li
         if (curNode->IsHTMLElement(nsGkAtoms::p)) {
           listItem =
             HTMLEditorRef().ReplaceContainerWithTransaction(
-                              *curNode->AsElement(), *itemType);
+                              *curNode->AsElement(), aItemType);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(!listItem)) {
             return NS_ERROR_FAILURE;
           }
         } else {
           listItem =
-            HTMLEditorRef().InsertContainerWithTransaction(*curNode, *itemType);
+            HTMLEditorRef().InsertContainerWithTransaction(*curNode, aItemType);
+          if (NS_WARN_IF(!CanHandleEditAction())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
           if (NS_WARN_IF(!listItem)) {
             return NS_ERROR_FAILURE;
           }
         }
         if (IsInlineNode(curNode)) {
           prevListItem = listItem;
         } else {
           prevListItem = nullptr;
@@ -4228,16 +4310,19 @@ HTMLEditRules::WillMakeList(const nsAStr
     } else {
       listItem = curNode->AsElement();
     }
 
     if (listItem) {
       // if we made a new list item, deal with it: tuck the listItem into the
       // end of the active list
       rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*listItem, *curList);
+      if (NS_WARN_IF(!CanHandleEditAction())) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
   }
 
   return NS_OK;
 }
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -316,21 +316,25 @@ protected:
    * @return                        Sets true to handled if this actually moves
    *                                the nodes.
    *                                canceled is always false.
    */
   EditActionResult MoveContents(Element& aElement, Element& aDestElement,
                                 int32_t* aInOutDestOffset);
 
   nsresult DeleteNonTableElements(nsINode* aNode);
-  nsresult WillMakeList(const nsAString* aListType,
-                        bool aEntireList,
-                        const nsAString* aBulletType,
-                        bool* aCancel, bool* aHandled,
-                        const nsAString* aItemType = nullptr);
+
+  /**
+   * XXX Should document what this does.
+   */
+  MOZ_MUST_USE nsresult
+  WillMakeList(const nsAString* aListType, bool aEntireList,
+               const nsAString* aBulletType,
+               bool* aCancel, bool* aHandled,
+               const nsAString* aItemType = nullptr);
 
   /**
    * Called before removing a list element.  This method actually removes
    * list elements and list item elements at Selection.  And move contents
    * in them where the removed list was.
    *
    * @param aCancel             Returns true if the operation is canceled.
    * @param aHandled            Returns true if the edit action is handled.
@@ -701,16 +705,26 @@ protected:
    *                                Otherwise, nullptr.
    */
   MOZ_MUST_USE SplitRangeOffFromNodeResult
   OutdentPartOfBlock(Element& aBlockElement,
                      nsIContent& aStartOfOutdent, nsIContent& aEndOutdent,
                      bool aIsBlockIndentedWithCSS);
 
   /**
+   * XXX Should document what this does.
+   * This method creates AutoSelectionRestorer.  Therefore, each caller
+   * need to check if the editor is still available even if this returns
+   * NS_OK.
+   */
+  MOZ_MUST_USE nsresult
+  MakeList(nsAtom& aListType, bool aEntireList, const nsAString* aBulletType,
+           bool* aCancel, nsAtom& aItemType);
+
+  /**
    * ConvertListType() replaces child list items of aListElement with
    * new list item element whose tag name is aNewListItemTag.
    * Note that if there are other list elements as children of aListElement,
    * this calls itself recursively even though it's invalid structure.
    *
    * @param aListElement        The list element whose list items will be
    *                            replaced.
    * @param aNewListTag         New list tag name.