Bug 1372829 - part1: Make mozilla::PlaceholderTransaction inherit mozilla::SupportsWeakPtr instead of nsSupportsWeakReference r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 14 Jun 2017 19:05:48 +0900
changeset 596708 e0285d6baadc31117ea40c42085b01aa091074b5
parent 596219 95543bdc59bd038a3d5d084b85a4fec493c349ee
child 596709 709a3cd34fb3eebe4fd40b8fcdde56bae59e43c2
push id64725
push usermasayuki@d-toybox.com
push dateMon, 19 Jun 2017 16:51:03 +0000
reviewersm_kato
bugs1372829
milestone56.0a1
Bug 1372829 - part1: Make mozilla::PlaceholderTransaction inherit mozilla::SupportsWeakPtr instead of nsSupportsWeakReference r?m_kato EditorBase stores PlaceholderTransaction with nsWeakPtr. However, retrieving its pointer requires a QI. So, for reducing the QI cost, EditorBase should use WeakPtr instead and PlaceholderTransaction needs to inherit SupportsWeakPtr instead of nsSupportsWeakReference. MozReview-Commit-ID: IYwSTbJebMk
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/PlaceholderTransaction.cpp
editor/libeditor/PlaceholderTransaction.h
editor/libeditor/TextEditor.cpp
editor/libeditor/nsIAbsorbingTransaction.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -121,22 +121,22 @@ namespace mozilla {
 using namespace dom;
 using namespace widget;
 
 /*****************************************************************************
  * mozilla::EditorBase
  *****************************************************************************/
 
 EditorBase::EditorBase()
-  : mPlaceHolderName(nullptr)
+  : mPlaceholderName(nullptr)
   , mSelState(nullptr)
   , mModCount(0)
   , mFlags(0)
   , mUpdateCount(0)
-  , mPlaceHolderBatch(0)
+  , mPlaceholderBatch(0)
   , mAction(EditAction::none)
   , mIMETextOffset(0)
   , mIMETextLength(0)
   , mDirection(eNone)
   , mDocDirtyState(-1)
   , mSpellcheckCheckboxState(eTriUnset)
   , mShouldTxnSetSelection(true)
   , mDidPreDestroy(false)
@@ -675,39 +675,39 @@ EditorBase::GetSelection(SelectionType a
   }
 
   return sel->AsSelection();
 }
 
 NS_IMETHODIMP
 EditorBase::DoTransaction(nsITransaction* aTxn)
 {
-  if (mPlaceHolderBatch && !mPlaceHolderTxn) {
-    nsCOMPtr<nsIAbsorbingTransaction> placeholderTransaction =
-      new PlaceholderTransaction(*this, mPlaceHolderName, Move(mSelState));
+  if (mPlaceholderBatch && !mPlaceholderTransactionWeak) {
+    RefPtr<PlaceholderTransaction> placeholderTransaction =
+      new PlaceholderTransaction(*this, mPlaceholderName, Move(mSelState));
 
     // Save off weak reference to placeholder transaction
-    mPlaceHolderTxn = do_GetWeakReference(placeholderTransaction);
-
-    // QI to an nsITransaction since that's what DoTransaction() expects
-    nsCOMPtr<nsITransaction> transaction =
-      do_QueryInterface(placeholderTransaction);
+    mPlaceholderTransactionWeak = placeholderTransaction;
+
     // We will recurse, but will not hit this case in the nested call
-    DoTransaction(transaction);
+    DoTransaction(placeholderTransaction);
 
     if (mTxnMgr) {
-      nsCOMPtr<nsITransaction> topTxn = mTxnMgr->PeekUndoStack();
-      if (topTxn) {
-        placeholderTransaction = do_QueryInterface(topTxn);
-        if (placeholderTransaction) {
+      nsCOMPtr<nsITransaction> topTransaction = mTxnMgr->PeekUndoStack();
+      nsCOMPtr<nsIAbsorbingTransaction> topAbsorbingTransaction =
+        do_QueryInterface(topTransaction);
+      if (topAbsorbingTransaction) {
+        RefPtr<PlaceholderTransaction> topPlaceholderTransaction =
+          topAbsorbingTransaction->AsPlaceholderTransaction();
+        if (topPlaceholderTransaction) {
           // there is a placeholder transaction on top of the undo stack.  It
           // is either the one we just created, or an earlier one that we are
           // now merging into.  From here on out remember this placeholder
           // instead of the one we just created.
-          mPlaceHolderTxn = do_GetWeakReference(placeholderTransaction);
+          mPlaceholderTransactionWeak = topPlaceholderTransaction;
         }
       }
     }
   }
 
   if (aTxn) {
     // XXX: Why are we doing selection specific batching stuff here?
     // XXX: Most entry points into the editor have auto variables that
@@ -911,47 +911,48 @@ EditorBase::EndTransaction()
 // a placeholder transaction to wrap up any further transaction
 // while the batch is open.  The advantage of this is that
 // placeholder transactions can later merge, if needed.  Merging
 // is unavailable between transaction manager batches.
 
 NS_IMETHODIMP
 EditorBase::BeginPlaceHolderTransaction(nsIAtom* aName)
 {
-  NS_PRECONDITION(mPlaceHolderBatch >= 0, "negative placeholder batch count!");
-  if (!mPlaceHolderBatch) {
+  MOZ_ASSERT(mPlaceholderBatch >= 0, "negative placeholder batch count!");
+  if (!mPlaceholderBatch) {
     NotifyEditorObservers(eNotifyEditorObserversOfBefore);
     // time to turn on the batch
     BeginUpdateViewBatch();
-    mPlaceHolderTxn = nullptr;
-    mPlaceHolderName = aName;
+    mPlaceholderTransactionWeak = nullptr;
+    mPlaceholderName = aName;
     RefPtr<Selection> selection = GetSelection();
     if (selection) {
       mSelState = MakeUnique<SelectionState>();
       mSelState->SaveSelection(selection);
       // Composition transaction can modify multiple nodes and it merges text
       // node for ime into single text node.
       // So if current selection is into IME text node, it might be failed
       // to restore selection by UndoTransaction.
       // So we need update selection by range updater.
-      if (mPlaceHolderName == nsGkAtoms::IMETxnName) {
+      if (mPlaceholderName == nsGkAtoms::IMETxnName) {
         mRangeUpdater.RegisterSelectionState(*mSelState);
       }
     }
   }
-  mPlaceHolderBatch++;
+  mPlaceholderBatch++;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::EndPlaceHolderTransaction()
 {
-  NS_PRECONDITION(mPlaceHolderBatch > 0, "zero or negative placeholder batch count when ending batch!");
-  if (mPlaceHolderBatch == 1) {
+  MOZ_ASSERT(mPlaceholderBatch > 0,
+             "zero or negative placeholder batch count when ending batch!");
+  if (mPlaceholderBatch == 1) {
     RefPtr<Selection> selection = GetSelection();
 
     // By making the assumption that no reflow happens during the calls
     // to EndUpdateViewBatch and ScrollSelectionIntoView, we are able to
     // allow the selection to cache a frame offset which is used by the
     // caret drawing code. We only enable this cache here; at other times,
     // we have no way to know whether reflow invalidates it
     // See bugs 35296 and 199412.
@@ -981,41 +982,36 @@ EditorBase::EndPlaceHolderTransaction()
     // cached for frame offset are Not available now
     if (selection) {
       selection->SetCanCacheFrameOffset(false);
     }
 
     if (mSelState) {
       // we saved the selection state, but never got to hand it to placeholder
       // (else we ould have nulled out this pointer), so destroy it to prevent leaks.
-      if (mPlaceHolderName == nsGkAtoms::IMETxnName) {
+      if (mPlaceholderName == nsGkAtoms::IMETxnName) {
         mRangeUpdater.DropSelectionState(*mSelState);
       }
       mSelState = nullptr;
     }
     // We might have never made a placeholder if no action took place.
-    if (mPlaceHolderTxn) {
-      nsCOMPtr<nsIAbsorbingTransaction> plcTxn = do_QueryReferent(mPlaceHolderTxn);
-      if (plcTxn) {
-        plcTxn->EndPlaceHolderBatch();
-      } else {
-        // in the future we will check to make sure undo is off here,
-        // since that is the only known case where the placeholdertxn would disappear on us.
-        // For now just removing the assert.
-      }
+    if (mPlaceholderTransactionWeak) {
+      RefPtr<PlaceholderTransaction> placeholderTransaction =
+        mPlaceholderTransactionWeak.get();
+      placeholderTransaction->EndPlaceHolderBatch();
       // notify editor observers of action but if composing, it's done by
       // compositionchange event handler.
       if (!mComposition) {
         NotifyEditorObservers(eNotifyEditorObserversOfEnd);
       }
     } else {
       NotifyEditorObservers(eNotifyEditorObserversOfCancel);
     }
   }
-  mPlaceHolderBatch--;
+  mPlaceholderBatch--;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::ShouldTxnSetSelection(bool* aResult)
 {
   NS_ENSURE_TRUE(aResult, NS_ERROR_NULL_POINTER);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -4,18 +4,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_EditorBase_h
 #define mozilla_EditorBase_h
 
 #include "mozilla/Assertions.h"         // for MOZ_ASSERT, etc.
 #include "mozilla/OwningNonNull.h"      // for OwningNonNull
 #include "mozilla/SelectionState.h"     // for RangeUpdater, etc.
-#include "mozilla/StyleSheet.h"   // for StyleSheet
+#include "mozilla/StyleSheet.h"         // for StyleSheet
 #include "mozilla/UniquePtr.h"
+#include "mozilla/WeakPtr.h"            // for WeakPtr
 #include "mozilla/dom/Text.h"
 #include "nsCOMPtr.h"                   // for already_AddRefed, nsCOMPtr
 #include "nsCycleCollectionParticipant.h"
 #include "nsGkAtoms.h"
 #include "nsIEditor.h"                  // for nsIEditor, etc.
 #include "nsIObserver.h"                // for NS_DECL_NSIOBSERVER, etc.
 #include "nsIPlaintextEditor.h"         // for nsIPlaintextEditor, etc.
 #include "nsISelectionController.h"     // for nsISelectionController constants
@@ -111,16 +112,17 @@ class DeleteNodeTransaction;
 class DeleteTextTransaction;
 class EditAggregateTransaction;
 class EditTransactionBase;
 class ErrorResult;
 class HTMLEditor;
 class InsertNodeTransaction;
 class InsertTextTransaction;
 class JoinNodeTransaction;
+class PlaceholderTransaction;
 class RemoveStyleSheetTransaction;
 class SetTextTransaction;
 class SplitNodeTransaction;
 class TextComposition;
 class TextEditor;
 struct EditorDOMPoint;
 
 namespace dom {
@@ -1003,21 +1005,21 @@ protected:
   // Current IME text node.
   RefPtr<Text> mIMETextNode;
   // The form field as an event receiver.
   nsCOMPtr<dom::EventTarget> mEventTarget;
   nsCOMPtr<nsIDOMEventListener> mEventListener;
   // Weak reference to the nsISelectionController.
   nsWeakPtr mSelConWeak;
   // Weak reference to placeholder for begin/end batch purposes.
-  nsWeakPtr mPlaceHolderTxn;
+  WeakPtr<PlaceholderTransaction> mPlaceholderTransactionWeak;
   // Weak reference to the nsIDOMDocument.
   nsWeakPtr mDocWeak;
   // Name of placeholder transaction.
-  nsIAtom* mPlaceHolderName;
+  nsIAtom* mPlaceholderName;
   // Saved selection state for placeholder transaction batching.
   mozilla::UniquePtr<SelectionState> mSelState;
   // IME composition this is not null between compositionstart and
   // compositionend.
   RefPtr<TextComposition> mComposition;
 
   // Listens to all low level actions on the doc.
   typedef AutoTArray<OwningNonNull<nsIEditActionListener>, 5>
@@ -1040,17 +1042,17 @@ protected:
   // Number of modifications (for undo/redo stack).
   uint32_t mModCount;
   // Behavior flags. See nsIPlaintextEditor.idl for the flags we use.
   uint32_t mFlags;
 
   int32_t mUpdateCount;
 
   // Nesting count for batching.
-  int32_t mPlaceHolderBatch;
+  int32_t mPlaceholderBatch;
   // The current editor action.
   EditAction mAction;
 
   // Offset in text node where IME comp string begins.
   uint32_t mIMETextOffset;
   // The Length of the composition string or commit string.  If this is length
   // of commit string, the length is truncated by maxlength attribute.
   uint32_t mIMETextLength;
--- a/editor/libeditor/PlaceholderTransaction.cpp
+++ b/editor/libeditor/PlaceholderTransaction.cpp
@@ -51,17 +51,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
     ImplCycleCollectionTraverse(cb, *tmp->mStartSel, "mStartSel", 0);
   }
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEditorBase);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEndSel);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(PlaceholderTransaction)
   NS_INTERFACE_MAP_ENTRY(nsIAbsorbingTransaction)
-  NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
 NS_INTERFACE_MAP_END_INHERITING(EditAggregateTransaction)
 
 NS_IMPL_ADDREF_INHERITED(PlaceholderTransaction, EditAggregateTransaction)
 NS_IMPL_RELEASE_INHERITED(PlaceholderTransaction, EditAggregateTransaction)
 
 NS_IMETHODIMP
 PlaceholderTransaction::DoTransaction()
 {
--- a/editor/libeditor/PlaceholderTransaction.h
+++ b/editor/libeditor/PlaceholderTransaction.h
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef PlaceholderTransaction_h
 #define PlaceholderTransaction_h
 
 #include "EditAggregateTransaction.h"
 #include "mozilla/EditorUtils.h"
 #include "mozilla/UniquePtr.h"
+#include "mozilla/WeakPtr.h"
 #include "nsIAbsorbingTransaction.h"
 #include "nsIDOMNode.h"
 #include "nsCOMPtr.h"
 #include "nsWeakPtr.h"
 #include "nsWeakReference.h"
 
 namespace mozilla {
 
@@ -21,21 +22,24 @@ class CompositionTransaction;
 
 /**
  * An aggregate transaction that knows how to absorb all subsequent
  * transactions with the same name.  This transaction does not "Do" anything.
  * But it absorbs other transactions via merge, and can undo/redo the
  * transactions it has absorbed.
  */
 
-class PlaceholderTransaction final : public EditAggregateTransaction,
-                                     public nsIAbsorbingTransaction,
-                                     public nsSupportsWeakReference
+class PlaceholderTransaction final
+ : public EditAggregateTransaction
+ , public nsIAbsorbingTransaction
+ , public SupportsWeakPtr<PlaceholderTransaction>
 {
 public:
+  MOZ_DECLARE_WEAKREFERENCE_TYPENAME(PlaceholderTransaction)
+
   NS_DECL_ISUPPORTS_INHERITED
 
   PlaceholderTransaction(EditorBase& aEditorBase, nsIAtom* aName,
                          UniquePtr<SelectionState> aSelState);
 
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(PlaceholderTransaction,
                                            EditAggregateTransaction)
 // ------------ EditAggregateTransaction -----------------------
@@ -54,16 +58,21 @@ public:
 
   NS_IMETHOD EndPlaceHolderBatch() override;
 
   NS_IMETHOD ForwardEndBatchTo(
                nsIAbsorbingTransaction* aForwardingAddress) override;
 
   NS_IMETHOD Commit() override;
 
+  NS_IMETHOD_(PlaceholderTransaction*) AsPlaceholderTransaction() override
+  {
+    return this;
+  }
+
   nsresult RememberEndingSelection();
 
 protected:
   virtual ~PlaceholderTransaction();
 
   // Do we auto absorb any and all transaction?
   bool mAbsorb;
   nsWeakPtr mForwarding;
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -867,17 +867,17 @@ TextEditor::UpdateIMEComposition(WidgetC
   // NOTE: TextComposition should receive selection change notification before
   //       CompositionChangeEventHandlingMarker notifies TextComposition of the
   //       end of handling compositionchange event because TextComposition may
   //       need to ignore selection changes caused by composition.  Therefore,
   //       CompositionChangeEventHandlingMarker must be destroyed after a call
   //       of NotifiyEditorObservers(eNotifyEditorObserversOfEnd) or
   //       NotifiyEditorObservers(eNotifyEditorObserversOfCancel) which notifies
   //       TextComposition of a selection change.
-  MOZ_ASSERT(!mPlaceHolderBatch,
+  MOZ_ASSERT(!mPlaceholderBatch,
     "UpdateIMEComposition() must be called without place holder batch");
   TextComposition::CompositionChangeEventHandlingMarker
     compositionChangeEventHandlingMarker(mComposition, aCompsitionChangeEvent);
 
   NotifyEditorObservers(eNotifyEditorObserversOfBefore);
 
   RefPtr<nsCaret> caretP = ps->GetCaret();
 
--- a/editor/libeditor/nsIAbsorbingTransaction.h
+++ b/editor/libeditor/nsIAbsorbingTransaction.h
@@ -18,16 +18,17 @@ Transaction interface to outside world
     0x15b3, \
     0x11d2, \
     {0x93, 0x2e, 0x00, 0x80, 0x5f, 0x8a, 0xdd, 0x32} }
 
 class nsIAtom;
 
 namespace mozilla {
 class EditorBase;
+class PlaceholderTransaction;
 class SelectionState;
 } // namespace mozilla
 
 /**
  * A transaction interface mixin - for transactions that can support.
  * the placeholder absorbtion idiom.
  */
 class nsIAbsorbingTransaction  : public nsISupports{
@@ -40,15 +41,18 @@ public:
   NS_IMETHOD GetTxnName(nsIAtom **aName)=0;
 
   NS_IMETHOD StartSelectionEquals(mozilla::SelectionState* aSelState,
                                   bool* aResult) = 0;
 
   NS_IMETHOD ForwardEndBatchTo(nsIAbsorbingTransaction *aForwardingAddress)=0;
 
   NS_IMETHOD Commit()=0;
+
+  NS_IMETHOD_(mozilla::PlaceholderTransaction*)
+    AsPlaceholderTransaction() = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIAbsorbingTransaction,
                               NS_IABSORBINGTRANSACTION_IID)
 
 #endif // nsIAbsorbingTransaction_h__