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
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 11 May 2018 15:52:24 +0900
changeset 476186 0899d038b7f35c5103e9b8f56347327734c78fcf
parent 476185 d5afcbdbea9d47129110b4c9b0e1be956e8fa9da
child 476187 74c8cf6f92babcf7c155f32777d4e3c2935db427
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1460509
milestone62.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 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