Bug 1460509 - part 2: Make TextEditRules::CreateBR() and TextEditRules::CreateMozBR() return both new <br> element node and error code since if they cause destroying the editor, each caller needs NS_ERROR_EDITOR_DESTROYED result r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 11 May 2018 15:52:24 +0900
changeset 798720 6aec242133f1f4385c0832b5bcb486b221145588
parent 798719 f035a55c225ed4ecc730daf466d7ee2392314b49
child 798721 7b2ba37b4f038296072318633aa749a9435962a1
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 2: Make TextEditRules::CreateBR() and TextEditRules::CreateMozBR() return both new <br> element node and error code since if they cause destroying the editor, each caller needs NS_ERROR_EDITOR_DESTROYED result r?m_kato First, this patch changes TextEditRules::CreateBRInternal() to a private method for making any callers use CreateBR() or CreateMozBR() instead. Then, this patch makes TextEditRules::CreateBRInternal() return both nsresult and created <br> element with CreateElementResult class. Finally, this patch makes all callers of them check if they don't return an error code including NS_ERROR_EDITOR_DESTROYED. MozReview-Commit-ID: 18OvPmbDVHK
editor/libeditor/EditorUtils.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/TextEditRules.cpp
editor/libeditor/TextEditRules.h
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@@ -21,16 +21,21 @@ class nsAtom;
 class nsIContentIterator;
 class nsISimpleEnumerator;
 class nsITransferable;
 class nsRange;
 
 namespace mozilla {
 template <class T> class OwningNonNull;
 
+namespace dom {
+class Element;
+class Text;
+} // namespace dom
+
 /***************************************************************************
  * EditActionResult is useful to return multiple results of an editor
  * action handler without out params.
  * Note that when you return an anonymous instance from a method, you should
  * use EditActionIgnored(), EditActionHandled() or EditActionCanceled() for
  * easier to read.  In other words, EditActionResult should be used when
  * declaring return type of a method, being an argument or defined as a local
  * variable.
@@ -141,16 +146,71 @@ EditActionHandled(nsresult aRv = NS_OK)
  */
 inline EditActionResult
 EditActionCanceled(nsresult aRv = NS_OK)
 {
   return EditActionResult(aRv, true, true);
 }
 
 /***************************************************************************
+ * CreateNodeResultBase is a simple class for CreateSomething() methods
+ * which want to return new node.
+ */
+template<typename NodeType>
+class CreateNodeResultBase;
+
+typedef CreateNodeResultBase<dom::Element> CreateElementResult;
+
+template<typename NodeType>
+class MOZ_STACK_CLASS CreateNodeResultBase final
+{
+  typedef CreateNodeResultBase<NodeType> SelfType;
+public:
+  bool Succeeded() const { return NS_SUCCEEDED(mRv); }
+  bool Failed() const { return NS_FAILED(mRv); }
+  nsresult Rv() const { return mRv; }
+  NodeType* GetNewNode() const { return mNode; }
+
+  CreateNodeResultBase() = delete;
+
+  explicit CreateNodeResultBase(nsresult aRv)
+    : mRv(aRv)
+  {
+    MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(mRv));
+  }
+
+  explicit CreateNodeResultBase(NodeType* aNode)
+    : mNode(aNode)
+    , mRv(aNode ? NS_OK : NS_ERROR_FAILURE)
+  {
+  }
+
+  explicit CreateNodeResultBase(already_AddRefed<NodeType>&& aNode)
+    : mNode(aNode)
+    , mRv(mNode.get() ? NS_OK : NS_ERROR_FAILURE)
+  {
+  }
+
+  CreateNodeResultBase(const SelfType& aOther) = delete;
+  SelfType& operator=(const SelfType& aOther) = delete;
+  CreateNodeResultBase(SelfType&& aOther) = default;
+  SelfType& operator=(SelfType&& aOther) = default;
+
+  already_AddRefed<NodeType> forget()
+  {
+    mRv = NS_ERROR_NOT_INITIALIZED;
+    return mNode.forget();
+  }
+
+private:
+  RefPtr<NodeType> mNode;
+  nsresult mRv;
+};
+
+/***************************************************************************
  * SplitNodeResult is a simple class for
  * EditorBase::SplitNodeDeepWithTransaction().
  * This makes the callers' code easier to read.
  */
 class MOZ_STACK_CLASS SplitNodeResult final
 {
 public:
   bool Succeeded() const { return NS_SUCCEEDED(mRv); }
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -5593,19 +5593,20 @@ HTMLEditRules::WillAlign(const nsAString
     }
     // Remember our new block for postprocessing
     mNewBlock = div;
     // Set up the alignment on the div, using HTML or CSS
     rv = AlignBlock(*div, aAlignType, ContentsOnly::yes);
     NS_ENSURE_SUCCESS(rv, rv);
     *aHandled = true;
     // Put in a moz-br so that it won't get deleted
-    RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(div, 0));
-    if (NS_WARN_IF(!brElement)) {
-      return NS_ERROR_FAILURE;
+    CreateElementResult createMozBrResult =
+      CreateMozBR(EditorRawDOMPoint(div, 0));
+    if (NS_WARN_IF(createMozBrResult.Failed())) {
+      return createMozBrResult.Rv();
     }
     EditorRawDOMPoint atStartOfDiv(div, 0);
     ErrorResult error;
     SelectionRef().Collapse(atStartOfDiv, error);
     // Don't restore the selection
     selectionRestorer.Abort();
     if (NS_WARN_IF(error.Failed())) {
       return error.StealNSResult();
@@ -7331,19 +7332,20 @@ HTMLEditRules::ReturnInHeader(Element& a
     MOZ_DIAGNOSTIC_ASSERT(
       HTMLEditUtils::IsHeader(*prevItem));
     bool isEmptyNode;
     rv = HTMLEditorRef().IsEmptyNode(prevItem, &isEmptyNode);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     if (isEmptyNode) {
-      RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(prevItem, 0));
-      if (NS_WARN_IF(!brElement)) {
-        return NS_ERROR_FAILURE;
+      CreateElementResult createMozBrResult =
+        CreateMozBR(EditorRawDOMPoint(prevItem, 0));
+      if (NS_WARN_IF(createMozBrResult.Failed())) {
+        return createMozBrResult.Rv();
       }
     }
   }
 
   // If the new (righthand) header node is empty, delete it
   if (IsEmptyBlockElement(aHeader, IgnoreSingleBR::eYes)) {
     rv = HTMLEditorRef().DeleteNodeWithTransaction(aHeader);
     if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -7799,19 +7801,20 @@ HTMLEditRules::ReturnInListItem(Element&
     HTMLEditorRef().GetPriorHTMLSibling(&aListItem);
   if (prevItem && HTMLEditUtils::IsListItem(prevItem)) {
     bool isEmptyNode;
     rv = HTMLEditorRef().IsEmptyNode(prevItem, &isEmptyNode);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     if (isEmptyNode) {
-      RefPtr<Element> brElement = CreateMozBR(EditorRawDOMPoint(prevItem, 0));
-      if (NS_WARN_IF(!brElement)) {
-        return NS_ERROR_FAILURE;
+      CreateElementResult createMozBrResult =
+        CreateMozBR(EditorRawDOMPoint(prevItem, 0));
+      if (NS_WARN_IF(createMozBrResult.Failed())) {
+        return createMozBrResult.Rv();
       }
     } else {
       rv = HTMLEditorRef().IsEmptyNode(&aListItem, &isEmptyNode, true);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       if (isEmptyNode) {
         RefPtr<nsAtom> nodeAtom = aListItem.NodeInfo()->NameAtom();
@@ -8553,18 +8556,20 @@ HTMLEditRules::AdjustSpecialBreaks()
   // Put moz-br's into these empty li's and td's
   for (auto& node : nodeArray) {
     // Need to put br at END of node.  It may have empty containers in it and
     // still pass the "IsEmptyNode" test, and we want the br's to be after
     // them.  Also, we want the br to be after the selection if the selection
     // is in this node.
     EditorRawDOMPoint endOfNode;
     endOfNode.SetToEndOf(node);
-    RefPtr<Element> brElement = CreateMozBR(endOfNode);
-    if (NS_WARN_IF(!brElement)) {
+    // XXX This method should return nsreuslt due to may be destroyed by this
+    //     CreateMozBr() call.
+    CreateElementResult createMozBrResult = CreateMozBR(endOfNode);
+    if (NS_WARN_IF(createMozBrResult.Failed())) {
       return;
     }
   }
 }
 
 nsresult
 HTMLEditRules::AdjustWhitespace()
 {
@@ -8777,19 +8782,19 @@ HTMLEditRules::AdjustSelection(nsIEditor
       if (point.GetContainer() == rootElement) {
         // Our root node is completely empty. Don't add a <br> here.
         // AfterEditInner() will add one for us when it calls
         // CreateBogusNodeIfNeeded()!
         return NS_OK;
       }
 
       // we know we can skip the rest of this routine given the cirumstance
-      RefPtr<Element> brElement = CreateMozBR(point);
-      if (NS_WARN_IF(!brElement)) {
-        return NS_ERROR_FAILURE;
+      CreateElementResult createMozBrResult = CreateMozBR(point);
+      if (NS_WARN_IF(createMozBrResult.Failed())) {
+        return createMozBrResult.Rv();
       }
       return NS_OK;
     }
   }
 
   // are we in a text node?
   if (point.IsInTextNode()) {
     return NS_OK; // we LIKE it when we are in a text node.  that RULZ
@@ -8807,21 +8812,21 @@ HTMLEditRules::AdjustSelection(nsIEditor
     RefPtr<Element> block = HTMLEditorRef().GetBlock(*point.GetContainer());
     RefPtr<Element> nearBlock = HTMLEditorRef().GetBlockNodeParent(nearNode);
     if (block && block == nearBlock) {
       if (nearNode && TextEditUtils::IsBreak(nearNode)) {
         if (!HTMLEditorRef().IsVisibleBRElement(nearNode)) {
           // need to insert special moz BR. Why?  Because if we don't
           // the user will see no new line for the break.  Also, things
           // like table cells won't grow in height.
-          RefPtr<Element> brElement = CreateMozBR(point);
-          if (NS_WARN_IF(!brElement)) {
-            return NS_ERROR_FAILURE;
+          CreateElementResult createMozBrResult = CreateMozBR(point);
+          if (NS_WARN_IF(createMozBrResult.Failed())) {
+            return createMozBrResult.Rv();
           }
-          point.Set(brElement);
+          point.Set(createMozBrResult.GetNewNode());
           // selection stays *before* moz-br, sticking to it
           ErrorResult error;
           SelectionRef().SetInterlinePosition(true, error);
           NS_WARNING_ASSERTION(!error.Failed(),
             "Failed to set interline position");
           error = NS_OK;
           SelectionRef().Collapse(point, error);
           if (NS_WARN_IF(error.Failed())) {
@@ -9445,20 +9450,21 @@ HTMLEditRules::InsertBRIfNeededInternal(
   nsresult rv = HTMLEditorRef().IsEmptyNode(&aNode, &isEmpty);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   if (!isEmpty) {
     return NS_OK;
   }
 
-  RefPtr<Element> brElement =
-    CreateBRInternal(EditorRawDOMPoint(&aNode, 0), aInsertMozBR);
-  if (NS_WARN_IF(!brElement)) {
-    return NS_ERROR_FAILURE;
+  CreateElementResult createBrResult =
+    !aInsertMozBR ? CreateBR(EditorRawDOMPoint(&aNode, 0)) :
+                    CreateMozBR(EditorRawDOMPoint(&aNode, 0));
+  if (NS_WARN_IF(createBrResult.Failed())) {
+    return createBrResult.Rv();
   }
   return NS_OK;
 }
 
 void
 HTMLEditRules::DidCreateNode(Selection& aSelection,
                              Element& aNewElement)
 {
--- a/editor/libeditor/TextEditRules.cpp
+++ b/editor/libeditor/TextEditRules.cpp
@@ -39,16 +39,23 @@
 #include "nsUnicharUtils.h"
 #include "nsIHTMLCollection.h"
 #include "nsPrintfCString.h"
 
 namespace mozilla {
 
 using namespace dom;
 
+template CreateElementResult
+TextEditRules::CreateBRInternal(const EditorDOMPoint& aPointToInsert,
+                                bool aCreateMozBR);
+template CreateElementResult
+TextEditRules::CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
+                                bool aCreateMozBR);
+
 #define CANCEL_OPERATION_IF_READONLY_OR_DISABLED \
   if (IsReadonly() || IsDisabled()) \
   {                     \
     *aCancel = true; \
     return NS_OK;       \
   };
 
 /********************************************************
@@ -1374,19 +1381,19 @@ TextEditRules::CreateTrailingBRIfNeeded(
   if (NS_WARN_IF(!lastChild)) {
     return NS_ERROR_FAILURE;
   }
 
   if (!lastChild->IsHTMLElement(nsGkAtoms::br)) {
     AutoTransactionsConserveSelection dontChangeMySelection(&TextEditorRef());
     EditorRawDOMPoint endOfRoot;
     endOfRoot.SetToEndOf(rootElement);
-    RefPtr<Element> brElement = CreateMozBR(endOfRoot);
-    if (NS_WARN_IF(!brElement)) {
-      return NS_ERROR_FAILURE;
+    CreateElementResult createMozBrResult = CreateMozBR(endOfRoot);
+    if (NS_WARN_IF(createMozBrResult.Failed())) {
+      return createMozBrResult.Rv();
     }
     return NS_OK;
   }
 
   // Check to see if the trailing BR is a former bogus node - this will have
   // stuck around if we previously morphed a trailing node into a bogus node.
   if (!TextEditorRef().IsMozEditorBogusNode(lastChild)) {
     return NS_OK;
@@ -1658,47 +1665,56 @@ TextEditRules::FillBufWithPWChars(nsAStr
   char16_t passwordChar = LookAndFeel::GetPasswordCharacter();
 
   aOutString->Truncate();
   for (int32_t i = 0; i < aLength; i++) {
     aOutString->Append(passwordChar);
   }
 }
 
-already_AddRefed<Element>
-TextEditRules::CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
-                                bool aCreateMozBR)
+template<typename PT, typename CT>
+CreateElementResult
+TextEditRules::CreateBRInternal(
+                 const EditorDOMPointBase<PT, CT>& aPointToInsert,
+                 bool aCreateMozBR)
 {
   MOZ_ASSERT(IsEditorDataAvailable());
 
   if (NS_WARN_IF(!aPointToInsert.IsSet())) {
-    return nullptr;
+    return CreateElementResult(NS_ERROR_FAILURE);
   }
 
   RefPtr<Element> brElement =
     TextEditorRef().InsertBrElementWithTransaction(SelectionRef(),
                                                    aPointToInsert);
+  if (NS_WARN_IF(!CanHandleEditAction())) {
+    return CreateElementResult(NS_ERROR_EDITOR_DESTROYED);
+  }
   if (NS_WARN_IF(!brElement)) {
-    return nullptr;
+    return CreateElementResult(NS_ERROR_FAILURE);
   }
 
   // give it special moz attr
-  if (aCreateMozBR) {
-    // XXX Why do we need to set this attribute with transaction?
-    nsresult rv =
-      TextEditorRef().SetAttributeWithTransaction(*brElement, *nsGkAtoms::type,
-                                                  NS_LITERAL_STRING("_moz"));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      // XXX Don't we need to remove the new <br> element from the DOM tree
-      //     in this case?
-      return nullptr;
-    }
+  if (!aCreateMozBR) {
+    return CreateElementResult(brElement.forget());
   }
 
-  return brElement.forget();
+  // XXX Why do we need to set this attribute with transaction?
+  nsresult rv =
+    TextEditorRef().SetAttributeWithTransaction(*brElement, *nsGkAtoms::type,
+                                                NS_LITERAL_STRING("_moz"));
+  // XXX Don't we need to remove the new <br> element from the DOM tree
+  //     in these case?
+  if (NS_WARN_IF(!CanHandleEditAction())) {
+    return CreateElementResult(NS_ERROR_EDITOR_DESTROYED);
+  }
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return CreateElementResult(NS_ERROR_FAILURE);
+  }
+  return CreateElementResult(brElement.forget());
 }
 
 nsresult
 TextEditRules::DocumentModified()
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
--- a/editor/libeditor/TextEditRules.h
+++ b/editor/libeditor/TextEditRules.h
@@ -3,16 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_TextEditRules_h
 #define mozilla_TextEditRules_h
 
 #include "mozilla/EditAction.h"
 #include "mozilla/EditorDOMPoint.h"
+#include "mozilla/EditorUtils.h"
 #include "mozilla/HTMLEditor.h" // for nsIEditor::AsHTMLEditor()
 #include "mozilla/TextEditor.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsIEditor.h"
 #include "nsINamed.h"
 #include "nsISupportsImpl.h"
 #include "nsITimer.h"
@@ -204,53 +205,55 @@ protected:
    */
   void RemoveIMETextFromPWBuf(uint32_t& aStart, nsAString* aIMEString);
 
   /**
    * Create a normal <br> element and insert it to aPointToInsert.
    *
    * @param aPointToInsert  The point where the new <br> element will be
    *                        inserted.
-   * @return                Returns created <br> element.
+   * @return                Returns created <br> element or an error code
+   *                        if couldn't create new <br> element.
    */
   template<typename PT, typename CT>
-  already_AddRefed<Element>
+  CreateElementResult
   CreateBR(const EditorDOMPointBase<PT, CT>& aPointToInsert)
   {
-    return CreateBRInternal(aPointToInsert, false);
+    CreateElementResult ret = CreateBRInternal(aPointToInsert, false);
+#ifdef DEBUG
+    // If editor is destroyed, it must return NS_ERROR_EDITOR_DESTROYED.
+    if (!CanHandleEditAction()) {
+      MOZ_ASSERT(ret.Rv() == NS_ERROR_EDITOR_DESTROYED);
+    }
+#endif // #ifdef DEBUG
+    return ret;
   }
 
   /**
    * Create a moz-<br> element and insert it to aPointToInsert.
    *
    * @param aPointToInsert  The point where the new moz-<br> element will be
    *                        inserted.
-   * @return                Returns created moz-<br> element.
+   * @return                Returns created <br> element or an error code
+   *                        if couldn't create new <br> element.
    */
   template<typename PT, typename CT>
-  already_AddRefed<Element>
+  CreateElementResult
   CreateMozBR(const EditorDOMPointBase<PT, CT>& aPointToInsert)
   {
-    return CreateBRInternal(aPointToInsert, true);
+    CreateElementResult ret = CreateBRInternal(aPointToInsert, true);
+#ifdef DEBUG
+    // If editor is destroyed, it must return NS_ERROR_EDITOR_DESTROYED.
+    if (!CanHandleEditAction()) {
+      MOZ_ASSERT(ret.Rv() == NS_ERROR_EDITOR_DESTROYED);
+    }
+#endif // #ifdef DEBUG
+    return ret;
   }
 
-  /**
-   * Create a normal <br> element or a moz-<br> element and insert it to
-   * aPointToInsert.
-   *
-   * @param aParentToInsert     The point where the new <br> element will be
-   *                            inserted.
-   * @param aCreateMozBR        true if the caller wants to create a moz-<br>
-   *                            element.  Otherwise, false.
-   * @return                    Returns created <br> element.
-   */
-  already_AddRefed<Element>
-  CreateBRInternal(const EditorRawDOMPoint& aPointToInsert,
-                   bool aCreateMozBR);
-
   void UndefineCaretBidiLevel();
 
   nsresult CheckBidiLevelForDeletion(const EditorRawDOMPoint& aSelectionPoint,
                                      nsIEditor::EDirection aAction,
                                      bool* aCancel);
 
   nsresult HideLastPWInput();
 
@@ -262,16 +265,32 @@ protected:
   bool IsReadonly() const;
   bool IsDisabled() const;
   bool IsMailEditor() const;
   bool DontEchoPassword() const;
 
 private:
   TextEditor* MOZ_NON_OWNING_REF mTextEditor;
 
+  /**
+   * Create a normal <br> element or a moz-<br> element and insert it to
+   * aPointToInsert.
+   *
+   * @param aParentToInsert     The point where the new <br> element will be
+   *                            inserted.
+   * @param aCreateMozBR        true if the caller wants to create a moz-<br>
+   *                            element.  Otherwise, false.
+   * @return                    Returns created <br> element and error code.
+   *                            If it succeeded, never returns nullptr.
+   */
+  template<typename PT, typename CT>
+  CreateElementResult
+  CreateBRInternal(const EditorDOMPointBase<PT, CT>& aPointToInsert,
+                   bool aCreateMozBR);
+
 protected:
   /**
    * AutoSafeEditorData grabs editor instance and related instances during
    * handling an edit action.  When this is created, its pointer is set to
    * the mSafeData, and this guarantees the lifetime of grabbing objects
    * until it's destroyed.
    */
   class MOZ_STACK_CLASS AutoSafeEditorData