From f337b95b6059d252f74c4ddd31f99efd018fdd55 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 20 Jun 2017 00:55:00 +0900 Subject: [PATCH] Bug 1372829 - part2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr r=froydnj,m_kato mozilla::EditorBase stores nsISelectionController and nsIDocument with nsWeakPtr. However, nsWeakPtr requires QI to retrieve actual pointer and it makes some damage to the performance. If mozilla::WeakPter were available for them, it'd be great. However, it's not available with nsISelectionController nor nsIDocument because it's possible to implement SupportsWeakPtr only with their subclasses but the subclasses shouldn't be referred by editor. Therefore, this patch creates mozilla::CachedWeakPtr which stores both raw pointer (as cache) and nsWeakPtr and when its cache is requested, it checks if the object referred by nsWeakPtr is still alive. Additionally, this patch hides the members from subclasses of EditorBase for reducing the maintenance cost. MozReview-Commit-ID: FvtM7453Vv8 --HG-- extra : rebase_source : a524a8ea327c3993645fafa81db8aef65090f1e0 --- dom/base/FragmentOrElement.h | 1 + editor/libeditor/EditorBase.cpp | 215 ++++++++++++++++----------- editor/libeditor/EditorBase.h | 68 ++++++++- editor/libeditor/HTMLEditor.cpp | 171 ++++++++++++--------- editor/libeditor/HTMLTableEditor.cpp | 1 - xpcom/base/nsIWeakReference.idl | 5 + xpcom/base/nsWeakReference.cpp | 1 + 7 files changed, 295 insertions(+), 167 deletions(-) diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index 0e8c86f8cfa8..7e58d334bb90 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -61,6 +61,7 @@ public: // nsIWeakReference NS_DECL_NSIWEAKREFERENCE virtual size_t SizeOfOnlyThis(mozilla::MallocSizeOf aMallocSizeOf) const override; + virtual bool IsAlive() const override { return mNode != nullptr; } void NoticeNodeDestruction() { diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 903cf2cb181c..4687d3c60af2 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -38,6 +38,7 @@ #include "mozilla/TextComposition.h" // for TextComposition #include "mozilla/TextEvents.h" #include "mozilla/dom/Element.h" // for Element, nsINode::AsElement +#include "mozilla/dom/HTMLBodyElement.h" #include "mozilla/dom/Text.h" #include "mozilla/dom/Event.h" #include "mozilla/mozalloc.h" // for operator new, etc. @@ -70,7 +71,6 @@ #include "nsIDOMMouseEvent.h" // for nsIDOMMouseEvent #include "nsIDOMNode.h" // for nsIDOMNode, etc. #include "nsIDOMNodeList.h" // for nsIDOMNodeList -#include "nsIDocument.h" // for nsIDocument #include "nsIDocumentStateListener.h" // for nsIDocumentStateListener #include "nsIEditActionListener.h" // for nsIEditActionListener #include "nsIEditorObserver.h" // for nsIEditorObserver @@ -150,7 +150,8 @@ EditorBase::EditorBase() EditorBase::~EditorBase() { - NS_ASSERTION(!mDocWeak || mDidPreDestroy, "Why PreDestroy hasn't been called?"); + MOZ_ASSERT(!IsInitialized() || mDidPreDestroy, + "Why PreDestroy hasn't been called?"); if (mComposition) { mComposition->OnEditorDestroyed(); @@ -208,20 +209,21 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(EditorBase) NS_IMETHODIMP -EditorBase::Init(nsIDOMDocument* aDoc, +EditorBase::Init(nsIDOMDocument* aDOMDocument, nsIContent* aRoot, - nsISelectionController* aSelCon, + nsISelectionController* aSelectionController, uint32_t aFlags, const nsAString& aValue) { MOZ_ASSERT(mAction == EditAction::none, "Initializing during an edit action is an error"); - MOZ_ASSERT(aDoc); - if (!aDoc) + MOZ_ASSERT(aDOMDocument); + if (!aDOMDocument) { return NS_ERROR_NULL_POINTER; + } // First only set flags, but other stuff shouldn't be initialized now. - // Don't move this call after initializing mDocWeak. + // Don't move this call after initializing mDocumentWeak. // SetFlags() can check whether it's called during initialization or not by // them. Note that SetFlags() will be called by PostCreate(). #ifdef DEBUG @@ -230,19 +232,21 @@ EditorBase::Init(nsIDOMDocument* aDoc, SetFlags(aFlags); NS_ASSERTION(NS_SUCCEEDED(rv), "SetFlags() failed"); - mDocWeak = do_GetWeakReference(aDoc); // weak reference to doc + nsCOMPtr document = do_QueryInterface(aDOMDocument); + mDocumentWeak = document.get(); // HTML editors currently don't have their own selection controller, // so they'll pass null as aSelCon, and we'll get the selection controller // off of the presshell. - nsCOMPtr selCon; - if (aSelCon) { - mSelConWeak = do_GetWeakReference(aSelCon); // weak reference to selectioncontroller - selCon = aSelCon; + nsCOMPtr selectionController; + if (aSelectionController) { + mSelectionControllerWeak = aSelectionController; + selectionController = aSelectionController; } else { nsCOMPtr presShell = GetPresShell(); - selCon = do_QueryInterface(presShell); + selectionController = do_QueryInterface(presShell); } - NS_ASSERTION(selCon, "Selection controller should be available at this point"); + MOZ_ASSERT(selectionController, + "Selection controller should be available at this point"); //set up root element if we are passed one. if (aRoot) @@ -259,13 +263,14 @@ EditorBase::Init(nsIDOMDocument* aDoc, mIMETextNode = nullptr; } - /* Show the caret */ - selCon->SetCaretReadOnly(false); - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); + // Show the caret. + selectionController->SetCaretReadOnly(false); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); + // Show all the selection reflected to user. + selectionController->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); - selCon->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL);//we want to see all the selection reflected to user - - NS_POSTCONDITION(mDocWeak, "bad state"); + MOZ_ASSERT(IsInitialized()); // Make sure that the editor will be destroyed properly mDidPreDestroy = false; @@ -344,8 +349,9 @@ EditorBase::CreateEventListeners() nsresult EditorBase::InstallEventListeners() { - NS_ENSURE_TRUE(mDocWeak && mEventListener, - NS_ERROR_NOT_INITIALIZED); + if (NS_WARN_IF(!IsInitialized()) || NS_WARN_IF(!mEventListener)) { + return NS_ERROR_NOT_INITIALIZED; + } // Initialize the event target. nsCOMPtr rootContent = GetRoot(); @@ -366,7 +372,7 @@ EditorBase::InstallEventListeners() void EditorBase::RemoveEventListeners() { - if (!mDocWeak || !mEventListener) { + if (!IsInitialized() || !mEventListener) { return; } reinterpret_cast(mEventListener.get())->Disconnect(); @@ -489,7 +495,7 @@ EditorBase::SetFlags(uint32_t aFlags) bool spellcheckerWasEnabled = CanEnableSpellCheck(); mFlags = aFlags; - if (!mDocWeak) { + if (!IsInitialized()) { // If we're initializing, we shouldn't do anything now. // SetFlags() will be called by PostCreate(), // we should synchronize some stuff for the flags at that time. @@ -555,17 +561,15 @@ EditorBase::GetIsDocumentEditable(bool* aIsDocumentEditable) already_AddRefed EditorBase::GetDocument() { - NS_PRECONDITION(mDocWeak, "bad state, mDocWeak weak pointer not initialized"); - nsCOMPtr doc = do_QueryReferent(mDocWeak); - return doc.forget(); + nsCOMPtr document = mDocumentWeak.get(); + return document.forget(); } already_AddRefed EditorBase::GetDOMDocument() { - NS_PRECONDITION(mDocWeak, "bad state, mDocWeak weak pointer not initialized"); - nsCOMPtr doc = do_QueryReferent(mDocWeak); - return doc.forget(); + nsCOMPtr domDocument = do_QueryInterface(mDocumentWeak); + return domDocument.forget(); } NS_IMETHODIMP @@ -578,11 +582,12 @@ EditorBase::GetDocument(nsIDOMDocument** aDoc) already_AddRefed EditorBase::GetPresShell() { - NS_PRECONDITION(mDocWeak, "bad state, null mDocWeak"); - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, nullptr); - nsCOMPtr ps = doc->GetShell(); - return ps.forget(); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return nullptr; + } + nsCOMPtr presShell = document->GetShell(); + return presShell.forget(); } already_AddRefed @@ -628,14 +633,14 @@ EditorBase::GetSelectionController(nsISelectionController** aSel) already_AddRefed EditorBase::GetSelectionController() { - nsCOMPtr selCon; - if (mSelConWeak) { - selCon = do_QueryReferent(mSelConWeak); + nsCOMPtr selectionController; + if (mSelectionControllerWeak) { + selectionController = mSelectionControllerWeak.get(); } else { nsCOMPtr presShell = GetPresShell(); - selCon = do_QueryInterface(presShell); + selectionController = do_QueryInterface(presShell); } - return selCon.forget(); + return selectionController.forget(); } NS_IMETHODIMP @@ -1043,7 +1048,8 @@ EditorBase::GetDocumentIsEmpty(bool* aDocumentIsEmpty) NS_IMETHODIMP EditorBase::SelectAll() { - if (!mDocWeak) { + // XXX Why doesn't this check if the document is alive? + if (!IsInitialized()) { return NS_ERROR_NOT_INITIALIZED; } ForceCompositionEnd(); @@ -1056,7 +1062,8 @@ EditorBase::SelectAll() NS_IMETHODIMP EditorBase::BeginningOfDocument() { - if (!mDocWeak) { + // XXX Why doesn't this check if the document is alive? + if (!IsInitialized()) { return NS_ERROR_NOT_INITIALIZED; } @@ -1093,7 +1100,10 @@ EditorBase::BeginningOfDocument() NS_IMETHODIMP EditorBase::EndOfDocument() { - NS_ENSURE_TRUE(mDocWeak, NS_ERROR_NOT_INITIALIZED); + // XXX Why doesn't this check if the document is alive? + if (NS_WARN_IF(!IsInitialized())) { + return NS_ERROR_NOT_INITIALIZED; + } // get selection RefPtr selection = GetSelection(); @@ -1128,20 +1138,22 @@ EditorBase::GetDocumentModified(bool* outDocModified) NS_IMETHODIMP EditorBase::GetDocumentCharacterSet(nsACString& characterSet) { - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED); - - characterSet = doc->GetDocumentCharacterSet(); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return NS_ERROR_UNEXPECTED; + } + characterSet = document->GetDocumentCharacterSet(); return NS_OK; } NS_IMETHODIMP EditorBase::SetDocumentCharacterSet(const nsACString& characterSet) { - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED); - - doc->SetDocumentCharacterSet(characterSet); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return NS_ERROR_UNEXPECTED; + } + document->SetDocumentCharacterSet(characterSet); return NS_OK; } @@ -2007,12 +2019,17 @@ NS_IMETHODIMP EditorBase::DebugDumpContent() { #ifdef DEBUG - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, NS_ERROR_NOT_INITIALIZED); - - nsCOMPtrbodyElem; - doc->GetBody(getter_AddRefs(bodyElem)); - nsCOMPtr content = do_QueryInterface(bodyElem); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return NS_ERROR_NOT_INITIALIZED; + } + nsCOMPtr domHTMLDocument = do_QueryInterface(document); + if (NS_WARN_IF(!domHTMLDocument)) { + return NS_ERROR_NOT_INITIALIZED; + } + nsCOMPtr bodyElement; + domHTMLDocument->GetBody(getter_AddRefs(bodyElement)); + nsCOMPtr content = do_QueryInterface(bodyElement); if (content) { content->List(); } @@ -2312,18 +2329,20 @@ EditorBase::CloneAttributes(Element* aDest, nsresult EditorBase::ScrollSelectionIntoView(bool aScrollToAnchor) { - nsCOMPtr selCon; - if (NS_SUCCEEDED(GetSelectionController(getter_AddRefs(selCon))) && selCon) { - int16_t region = nsISelectionController::SELECTION_FOCUS_REGION; - - if (aScrollToAnchor) { - region = nsISelectionController::SELECTION_ANCHOR_REGION; - } - - selCon->ScrollSelectionIntoView(nsISelectionController::SELECTION_NORMAL, - region, nsISelectionController::SCROLL_OVERFLOW_HIDDEN); + nsCOMPtr selectionController = + GetSelectionController(); + if (!selectionController) { + return NS_OK; } + int16_t region = nsISelectionController::SELECTION_FOCUS_REGION; + if (aScrollToAnchor) { + region = nsISelectionController::SELECTION_ANCHOR_REGION; + } + selectionController->ScrollSelectionIntoView( + nsISelectionController::SELECTION_NORMAL, + region, + nsISelectionController::SCROLL_OVERFLOW_HIDDEN); return NS_OK; } @@ -4787,22 +4806,27 @@ EditorBase::InitializeSelection(nsIDOMEventTarget* aFocusEventTarget) nsCOMPtr presShell = GetPresShell(); NS_ENSURE_TRUE(presShell, NS_ERROR_NOT_INITIALIZED); - nsCOMPtr selCon; - nsresult rv = GetSelectionController(getter_AddRefs(selCon)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr selectionController = + GetSelectionController(); + if (NS_WARN_IF(!selectionController)) { + return NS_ERROR_FAILURE; + } // Init the caret RefPtr caret = presShell->GetCaret(); NS_ENSURE_TRUE(caret, NS_ERROR_UNEXPECTED); caret->SetIgnoreUserModify(false); caret->SetSelection(selection); - selCon->SetCaretReadOnly(IsReadonly()); - selCon->SetCaretEnabled(true); + selectionController->SetCaretReadOnly(IsReadonly()); + selectionController->SetCaretEnabled(true); // Init selection - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); - selCon->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); - selCon->RepaintSelection(nsISelectionController::SELECTION_NORMAL); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); + selectionController->SetSelectionFlags( + nsISelectionDisplay::DISPLAY_ALL); + selectionController->RepaintSelection( + nsISelectionController::SELECTION_NORMAL); // If the computed selection root isn't root content, we should set it // as selection ancestor limit. However, if that is root element, it means // there is not limitation of the selection, then, we must set nullptr. @@ -4852,9 +4876,11 @@ EditorBase::InitializeSelection(nsIDOMEventTarget* aFocusEventTarget) NS_IMETHODIMP EditorBase::FinalizeSelection() { - nsCOMPtr selCon; - nsresult rv = GetSelectionController(getter_AddRefs(selCon)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr selectionController = + GetSelectionController(); + if (NS_WARN_IF(!selectionController)) { + return NS_ERROR_FAILURE; + } RefPtr selection = GetSelection(); NS_ENSURE_STATE(selection); @@ -4864,7 +4890,7 @@ EditorBase::FinalizeSelection() nsCOMPtr presShell = GetPresShell(); NS_ENSURE_TRUE(presShell, NS_ERROR_NOT_INITIALIZED); - selCon->SetCaretEnabled(false); + selectionController->SetCaretEnabled(false); nsFocusManager* fm = nsFocusManager::GetFocusManager(); NS_ENSURE_TRUE(fm, NS_ERROR_NOT_INITIALIZED); @@ -4879,25 +4905,30 @@ EditorBase::FinalizeSelection() ErrorResult ret; if (!doc || !doc->HasFocus(ret)) { // If the document already lost focus, mark the selection as disabled. - selCon->SetDisplaySelection(nsISelectionController::SELECTION_DISABLED); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_DISABLED); } else { // Otherwise, mark selection as normal because outside of a // contenteditable element should be selected with normal selection // color after here. - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); } } else if (IsFormWidget() || IsPasswordEditor() || IsReadonly() || IsDisabled() || IsInputFiltered()) { // In or