From 32e8155127126c187ce32f7368742057bcaf69da Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:47:12 -0400 Subject: Bug 1372829 - Part 2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr Tag #1375 --- dom/base/FragmentOrElement.h | 1 + editor/libeditor/EditorBase.cpp | 214 ++++++++++++++++++++--------------- editor/libeditor/EditorBase.h | 68 ++++++++++- editor/libeditor/HTMLEditor.cpp | 160 +++++++++++++++----------- editor/libeditor/HTMLTableEditor.cpp | 1 - editor/libeditor/TextEditor.cpp | 2 +- xpcom/base/nsIWeakReference.idl | 5 + xpcom/glue/nsWeakReference.cpp | 1 + 8 files changed, 288 insertions(+), 164 deletions(-) diff --git a/dom/base/FragmentOrElement.h b/dom/base/FragmentOrElement.h index 4edd88908..07403cf39 100644 --- a/dom/base/FragmentOrElement.h +++ b/dom/base/FragmentOrElement.h @@ -62,6 +62,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 f068e9179..5df4ff2c4 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -37,6 +37,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. @@ -71,7 +72,6 @@ #include "nsIDOMNode.h" // for nsIDOMNode, etc. #include "nsIDOMNodeList.h" // for nsIDOMNodeList #include "nsIDOMText.h" // for nsIDOMText -#include "nsIDocument.h" // for nsIDocument #include "nsIDocumentStateListener.h" // for nsIDocumentStateListener #include "nsIEditActionListener.h" // for nsIEditActionListener #include "nsIEditorObserver.h" // for nsIEditorObserver @@ -151,7 +151,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(); @@ -220,20 +221,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 @@ -242,19 +244,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) @@ -271,13 +275,14 @@ EditorBase::Init(nsIDOMDocument* aDoc, mIMETextNode = nullptr; } - /* Show the caret */ - selCon->SetCaretReadOnly(false); - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); - - selCon->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL);//we want to see all the selection reflected to user + // Show the caret. + selectionController->SetCaretReadOnly(false); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); + // Show all the selection reflected to user. + selectionController->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); - NS_POSTCONDITION(mDocWeak, "bad state"); + MOZ_ASSERT(IsInitialized()); // Make sure that the editor will be destroyed properly mDidPreDestroy = false; @@ -356,8 +361,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(); @@ -378,7 +384,7 @@ EditorBase::InstallEventListeners() void EditorBase::RemoveEventListeners() { - if (!mDocWeak || !mEventListener) { + if (!IsInitialized() || !mEventListener) { return; } reinterpret_cast(mEventListener.get())->Disconnect(); @@ -501,7 +507,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. @@ -567,17 +573,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 @@ -590,11 +594,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 @@ -640,14 +645,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 @@ -1065,7 +1070,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(); @@ -1078,7 +1084,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; } @@ -1115,7 +1122,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(); @@ -1150,20 +1160,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; } @@ -2004,12 +2016,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(); } @@ -2323,18 +2340,20 @@ EditorBase::CloneAttributes(Element* aDest, NS_IMETHODIMP 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; } @@ -4784,22 +4803,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. @@ -4867,9 +4891,11 @@ private: 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); @@ -4879,7 +4905,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); @@ -4894,29 +4920,33 @@ 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