Bug 1457083 - part 2: Make all observer methods of HTMLEditRules create AutoSafeEditorData r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 26 Apr 2018 23:27:50 +0900
changeset 475869 fcacbe739f6779e1bdb51773c1480c9d3271c6ee
parent 475868 d7e797f898b05a4edc47b7369dad793162cb91e6
child 475870 4320b35386e96417f68d4862f6a369473750d029
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
bugs1457083
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 1457083 - part 2: Make all observer methods of HTMLEditRules create AutoSafeEditorData r=m_kato Except HTMLEditRules::WillJoinNodes(), observer methods of HTMLEditRules accesses mHTMLEditor. Therefore, we need to make them create AutoSafeEditorData instance in the stack. Note that for reducing EditorBase::GetSelection() calls, this patch adds Selection& argument to all of them. MozReview-Commit-ID: 6mjuD2gZwVp
editor/libeditor/EditorBase.cpp
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/TextEditor.cpp
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1384,19 +1384,24 @@ EditorBase::CreateNodeWithTransaction(
     // stores child node, it may not be at the offset since new element must
     // be inserted before the old child.  Although, mutation observer can do
     // anything, but currently, we don't check it.
     mRangeUpdater.SelAdjCreateNode(
                     EditorRawDOMPoint(aPointToInsert.GetContainer(),
                                       aPointToInsert.Offset()));
   }
 
-  if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidCreateNode(newElement);
+  if (mRules && mRules->AsHTMLEditRules() && newElement) {
+    Selection* selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->DidCreateNode(*selection, *newElement);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   if (!mActionListeners.IsEmpty()) {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->DidCreateNode(nsDependentAtomString(&aTagName),
                               GetAsDOMNode(newElement), rv);
     }
@@ -1440,18 +1445,23 @@ EditorBase::InsertNodeWithTransaction(
 
   RefPtr<InsertNodeTransaction> transaction =
     InsertNodeTransaction::Create(*this, aContentToInsert, aPointToInsert);
   nsresult rv = DoTransaction(transaction);
 
   mRangeUpdater.SelAdjInsertNode(aPointToInsert);
 
   if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidInsertNode(aContentToInsert);
+    Selection* selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->DidInsertNode(*selection, aContentToInsert);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   if (!mActionListeners.IsEmpty()) {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->DidInsertNode(aContentToInsert.AsDOMNode(), rv);
     }
   }
@@ -1508,19 +1518,25 @@ EditorBase::SplitNodeWithTransaction(
   nsCOMPtr<nsIContent> newNode = transaction->GetNewNode();
   NS_WARNING_ASSERTION(newNode, "Failed to create a new left node");
 
   // XXX Some other transactions manage range updater by themselves.
   //     Why doesn't SplitNodeTransaction do it?
   mRangeUpdater.SelAdjSplitNode(*aStartOfRightNode.GetContainerAsContent(),
                                 newNode);
 
-  if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidSplitNode(aStartOfRightNode.GetContainer(), newNode);
+  if (mRules && mRules->AsHTMLEditRules() && newNode) {
+    Selection* selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->DidSplitNode(*selection,
+                                  *aStartOfRightNode.GetContainer(), *newNode);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   if (mInlineSpellChecker) {
     RefPtr<mozInlineSpellChecker> spellChecker = mInlineSpellChecker;
     spellChecker->DidSplitNode(aStartOfRightNode.GetContainer(), newNode);
   }
 
   if (!mActionListeners.IsEmpty()) {
@@ -1578,18 +1594,23 @@ EditorBase::JoinNodesWithTransaction(nsI
   }
 
   // XXX Some other transactions manage range updater by themselves.
   //     Why doesn't JoinNodeTransaction do it?
   mRangeUpdater.SelAdjJoinNodes(aLeftNode, aRightNode, *parent, offset,
                                 (int32_t)oldLeftNodeLen);
 
   if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidJoinNodes(aLeftNode, aRightNode);
+    Selection* selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->DidJoinNodes(*selection, aLeftNode, aRightNode);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   if (mInlineSpellChecker) {
     RefPtr<mozInlineSpellChecker> spellChecker = mInlineSpellChecker;
     spellChecker->DidJoinNodes(aLeftNode, aRightNode);
   }
 
   if (mTextServicesDocument && NS_SUCCEEDED(rv)) {
@@ -1620,18 +1641,23 @@ EditorBase::DeleteNode(nsIDOMNode* aNode
 
 nsresult
 EditorBase::DeleteNodeWithTransaction(nsINode& aNode)
 {
   AutoRules beginRulesSniffing(this, EditAction::createNode,
                                nsIEditor::ePrevious);
 
   if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->WillDeleteNode(&aNode);
+    Selection* selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->WillDeleteNode(*selection, aNode);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   // FYI: DeleteNodeTransaction grabs aNode while it's alive.  So, it's safe
   //      to refer aNode even after calling DoTransaction().
   RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
     DeleteNodeTransaction::MaybeCreate(*this, aNode);
   nsresult rv = deleteNodeTransaction ? DoTransaction(deleteNodeTransaction) :
                                         NS_ERROR_FAILURE;
@@ -2771,20 +2797,25 @@ EditorBase::InsertTextIntoTextNodeWithTr
   }
 
   // XXX We may not need these view batches anymore.  This is handled at a
   // higher level now I believe.
   BeginUpdateViewBatch();
   nsresult rv = DoTransaction(transaction);
   EndUpdateViewBatch();
 
-  if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidInsertText(insertedTextNode, insertedOffset,
-                                 aStringToInsert);
+  if (mRules && mRules->AsHTMLEditRules() && insertedTextNode) {
+    Selection* selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->DidInsertText(*selection, *insertedTextNode,
+                                   insertedOffset, aStringToInsert);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   // let listeners know what happened
   if (!mActionListeners.IsEmpty()) {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->DidInsertText(insertedTextNode, insertedOffset,
                               aStringToInsert, rv);
@@ -2922,34 +2953,38 @@ EditorBase::SetTextImpl(Selection& aSele
   // the rules!
   ErrorResult res;
   aCharData.SetData(aString, res);
   nsresult rv = res.StealNSResult();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
+  RefPtr<Selection> selection = GetSelection();
+  if (NS_WARN_IF(!selection)) {
+    return NS_ERROR_FAILURE;
+  }
+
   {
     // Create a nested scope to not overwrite rv from the outer scope.
-    RefPtr<Selection> selection = GetSelection();
     DebugOnly<nsresult> rv = selection->Collapse(&aCharData, aString.Length());
     NS_ASSERTION(NS_SUCCEEDED(rv),
                  "Selection could not be collapsed after insert");
   }
 
   mRangeUpdater.SelAdjDeleteText(&aCharData, 0, length);
   mRangeUpdater.SelAdjInsertText(aCharData, 0, aString);
 
   if (mRules && mRules->AsHTMLEditRules()) {
     RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
     if (length) {
-      htmlEditRules->DidDeleteText(&aCharData, 0, length);
+      htmlEditRules->DidDeleteText(*selection, aCharData, 0, length);
     }
     if (!aString.IsEmpty()) {
-      htmlEditRules->DidInsertText(&aCharData, 0, aString);
+      htmlEditRules->DidInsertText(*selection, aCharData, 0, aString);
     }
   }
 
   // Let listeners know what happened
   if (!mActionListeners.IsEmpty()) {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       if (length) {
@@ -2984,18 +3019,23 @@ EditorBase::DeleteTextWithTransaction(Ch
     for (auto& listener : listeners) {
       listener->WillDeleteText(&aCharData, aOffset, aLength);
     }
   }
 
   nsresult rv = DoTransaction(transaction);
 
   if (mRules && mRules->AsHTMLEditRules()) {
-    RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidDeleteText(&aCharData, aOffset, aLength);
+    RefPtr<Selection> selection = GetSelection();
+    if (selection) {
+      RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
+      htmlEditRules->DidDeleteText(*selection, aCharData, aOffset, aLength);
+    } else {
+      NS_WARNING("Selection has gone");
+    }
   }
 
   // Let listeners know what happened
   if (!mActionListeners.IsEmpty()) {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->DidDeleteText(&aCharData, aOffset, aLength, rv);
     }
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -9312,73 +9312,99 @@ HTMLEditRules::InsertBRIfNeededInternal(
     CreateBRInternal(EditorRawDOMPoint(&aNode, 0), aInsertMozBR);
   if (NS_WARN_IF(!brElement)) {
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
 void
-HTMLEditRules::DidCreateNode(Element* aNewElement)
+HTMLEditRules::DidCreateNode(Selection& aSelection,
+                             Element& aNewElement)
 {
   if (!mListenerEnabled) {
     return;
   }
-  if (NS_WARN_IF(!aNewElement)) {
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
     return;
   }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
   // assumption that Join keeps the righthand node
   IgnoredErrorResult error;
-  mUtilRange->SelectNode(*aNewElement, error);
+  mUtilRange->SelectNode(aNewElement, error);
   if (NS_WARN_IF(error.Failed())) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
-HTMLEditRules::DidInsertNode(nsIContent& aContent)
+HTMLEditRules::DidInsertNode(Selection& aSelection,
+                             nsIContent& aContent)
 {
   if (!mListenerEnabled) {
     return;
   }
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return;
+  }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
   IgnoredErrorResult error;
   mUtilRange->SelectNode(aContent, error);
   if (NS_WARN_IF(error.Failed())) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
-HTMLEditRules::WillDeleteNode(nsINode* aChild)
+HTMLEditRules::WillDeleteNode(Selection& aSelection,
+                              nsINode& aChild)
 {
   if (!mListenerEnabled) {
     return;
   }
-  if (NS_WARN_IF(!aChild)) {
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
     return;
   }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
   IgnoredErrorResult error;
-  mUtilRange->SelectNode(*aChild, error);
+  mUtilRange->SelectNode(aChild, error);
   if (NS_WARN_IF(error.Failed())) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
-HTMLEditRules::DidSplitNode(nsINode* aExistingRightNode,
-                            nsINode* aNewLeftNode)
+HTMLEditRules::DidSplitNode(Selection& aSelection,
+                            nsINode& aExistingRightNode,
+                            nsINode& aNewLeftNode)
 {
   if (!mListenerEnabled) {
     return;
   }
-  nsresult rv = mUtilRange->SetStartAndEnd(aNewLeftNode, 0,
-                                           aExistingRightNode, 0);
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return;
+  }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
+  nsresult rv = mUtilRange->SetStartAndEnd(&aNewLeftNode, 0,
+                                           &aExistingRightNode, 0);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
 HTMLEditRules::WillJoinNodes(nsINode& aLeftNode,
@@ -9387,76 +9413,104 @@ HTMLEditRules::WillJoinNodes(nsINode& aL
   if (!mListenerEnabled) {
     return;
   }
   // remember split point
   mJoinOffset = aLeftNode.Length();
 }
 
 void
-HTMLEditRules::DidJoinNodes(nsINode& aLeftNode,
+HTMLEditRules::DidJoinNodes(Selection& aSelection,
+                            nsINode& aLeftNode,
                             nsINode& aRightNode)
 {
   if (!mListenerEnabled) {
     return;
   }
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return;
+  }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
   // assumption that Join keeps the righthand node
   nsresult rv = mUtilRange->CollapseTo(&aRightNode, mJoinOffset);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
-HTMLEditRules::DidInsertText(nsINode* aTextNode,
+HTMLEditRules::DidInsertText(Selection& aSelection,
+                             nsINode& aTextNode,
                              int32_t aOffset,
                              const nsAString& aString)
 {
   if (!mListenerEnabled) {
     return;
   }
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return;
+  }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
   int32_t length = aString.Length();
-  nsresult rv = mUtilRange->SetStartAndEnd(aTextNode, aOffset,
-                                           aTextNode, aOffset + length);
+  nsresult rv = mUtilRange->SetStartAndEnd(&aTextNode, aOffset,
+                                           &aTextNode, aOffset + length);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
-HTMLEditRules::DidDeleteText(nsINode* aTextNode,
+HTMLEditRules::DidDeleteText(Selection& aSelection,
+                             nsINode& aTextNode,
                              int32_t aOffset,
                              int32_t aLength)
 {
   if (!mListenerEnabled) {
     return;
   }
-  nsresult rv = mUtilRange->CollapseTo(aTextNode, aOffset);
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return;
+  }
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
+  nsresult rv = mUtilRange->CollapseTo(&aTextNode, aOffset);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
 }
 
 void
-HTMLEditRules::WillDeleteSelection(Selection* aSelection)
+HTMLEditRules::WillDeleteSelection(Selection& aSelection)
 {
   if (!mListenerEnabled) {
     return;
   }
-  if (NS_WARN_IF(!aSelection)) {
+
+  if (NS_WARN_IF(!mHTMLEditor)) {
     return;
   }
-  EditorRawDOMPoint startPoint = EditorBase::GetStartPoint(aSelection);
+
+  AutoSafeEditorData setData(*this, *mHTMLEditor, aSelection);
+
+  EditorRawDOMPoint startPoint = EditorBase::GetStartPoint(&aSelection);
   if (NS_WARN_IF(!startPoint.IsSet())) {
     return;
   }
-  EditorRawDOMPoint endPoint = EditorBase::GetEndPoint(aSelection);
+  EditorRawDOMPoint endPoint = EditorBase::GetEndPoint(&aSelection);
   if (NS_WARN_IF(!endPoint.IsSet())) {
     return;
   }
   nsresult rv = mUtilRange->SetStartAndEnd(startPoint, endPoint);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
   UpdateDocChangeRange(mUtilRange);
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -99,27 +99,31 @@ public:
 
   nsresult GetListState(bool* aMixed, bool* aOL, bool* aUL, bool* aDL);
   nsresult GetListItemState(bool* aMixed, bool* aLI, bool* aDT, bool* aDD);
   nsresult GetIndentState(bool* aCanIndent, bool* aCanOutdent);
   nsresult GetAlignment(bool* aMixed, nsIHTMLEditor::EAlignment* aAlign);
   nsresult GetParagraphState(bool* aMixed, nsAString& outFormat);
   nsresult MakeSureElemStartsAndEndsOnCR(nsINode& aNode);
 
-  void DidCreateNode(Element* aNewElement);
-  void DidInsertNode(nsIContent& aNode);
-  void WillDeleteNode(nsINode* aChild);
-  void DidSplitNode(nsINode* aExistingRightNode,
-                    nsINode* aNewLeftNode);
+  void DidCreateNode(Selection& aSelection, Element& aNewElement);
+  void DidInsertNode(Selection& aSelection, nsIContent& aNode);
+  void WillDeleteNode(Selection& aSelection, nsINode& aChild);
+  void DidSplitNode(Selection& aSelection,
+                    nsINode& aExistingRightNode,
+                    nsINode& aNewLeftNode);
   void WillJoinNodes(nsINode& aLeftNode, nsINode& aRightNode);
-  void DidJoinNodes(nsINode& aLeftNode, nsINode& aRightNode);
-  void DidInsertText(nsINode* aTextNode, int32_t aOffset,
+  void DidJoinNodes(Selection& aSelection,
+                    nsINode& aLeftNode, nsINode& aRightNode);
+  void DidInsertText(Selection& aSelection,
+                     nsINode& aTextNode, int32_t aOffset,
                      const nsAString& aString);
-  void DidDeleteText(nsINode* aTextNode, int32_t aOffset, int32_t aLength);
-  void WillDeleteSelection(Selection* aSelection);
+  void DidDeleteText(Selection& aSelection,
+                     nsINode& aTextNode, int32_t aOffset, int32_t aLength);
+  void WillDeleteSelection(Selection& aSelection);
 
   void StartToListenToEditActions() { mListenerEnabled = true; }
   void EndListeningToEditActions() { mListenerEnabled = false; }
 
 protected:
   virtual ~HTMLEditRules();
 
   HTMLEditor& HTMLEditorRef() const
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -745,20 +745,20 @@ TextEditor::DeleteSelectionWithTransacti
 
   RefPtr<CharacterData> deleteCharData =
     CharacterData::FromNodeOrNull(deleteNode);
   AutoRules beginRulesSniffing(this, EditAction::deleteSelection, aDirection);
 
   if (mRules && mRules->AsHTMLEditRules()) {
     if (!deleteNode) {
       RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-      htmlEditRules->WillDeleteSelection(selection);
+      htmlEditRules->WillDeleteSelection(*selection);
     } else if (!deleteCharData) {
       RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-      htmlEditRules->WillDeleteNode(deleteNode);
+      htmlEditRules->WillDeleteNode(*selection, *deleteNode);
     }
   }
 
   // Notify nsIEditActionListener::WillDelete[Selection|Text]
   if (!mActionListeners.IsEmpty()) {
     if (!deleteNode) {
       AutoActionListenerArray listeners(mActionListeners);
       for (auto& listener : listeners) {
@@ -771,18 +771,19 @@ TextEditor::DeleteSelectionWithTransacti
       }
     }
   }
 
   // Delete the specified amount
   nsresult rv = DoTransaction(deleteSelectionTransaction);
 
   if (mRules && mRules->AsHTMLEditRules() && deleteCharData) {
+    MOZ_ASSERT(deleteNode);
     RefPtr<HTMLEditRules> htmlEditRules = mRules->AsHTMLEditRules();
-    htmlEditRules->DidDeleteText(deleteNode, deleteCharOffset, 1);
+    htmlEditRules->DidDeleteText(*selection, *deleteNode, deleteCharOffset, 1);
   }
 
   if (mTextServicesDocument && NS_SUCCEEDED(rv) &&
       deleteNode && !deleteCharData) {
     RefPtr<TextServicesDocument> textServicesDocument = mTextServicesDocument;
     textServicesDocument->DidDeleteNode(deleteNode);
   }