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
--- 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__