From 09c6c60b8a566c266b57affce604a6c28de0c624 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Thu, 6 Apr 2017 13:11:52 +0900 Subject: [PATCH] Bug 1343642 - Ensure to grab nsFrameSelection before calling its methods unless calling only const methods. r=smaug MozReview-Commit-ID: 9GKujCcrhly --- dom/html/HTMLInputElement.cpp | 2 +- layout/base/PresShell.cpp | 70 ++++++++++++++++---------- layout/generic/nsSelection.cpp | 89 ++++++++++++++++++++-------------- 3 files changed, 98 insertions(+), 63 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 192911705017..775a12fccc7b 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -3592,7 +3592,7 @@ HTMLInputElement::Select() nsTextEditorState* tes = GetEditorState(); if (tes) { - nsFrameSelection* fs = tes->GetConstFrameSelection(); + RefPtr fs = tes->GetConstFrameSelection(); if (fs && fs->MouseDownRecorded()) { // This means that we're being called while the frame selection has a mouse // down event recorded to adjust the caret during the mouse up event. diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 2a70a7462f5b..0fb3a17d3a79 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -990,7 +990,8 @@ PresShell::Init(nsIDocument* aDocument, mSelection = new nsFrameSelection(); - mSelection->Init(this, nullptr); + RefPtr frameSelection = mSelection; + frameSelection->Init(this, nullptr); // Important: this has to happen after the selection has been set up #ifdef SHOW_CARET @@ -1283,7 +1284,8 @@ PresShell::Destroy() } if (mSelection) { - mSelection->DisconnectFromPresShell(); + RefPtr frameSelection = mSelection; + frameSelection->DisconnectFromPresShell(); } if (mAccessibleCaretEventHub) { @@ -1581,14 +1583,16 @@ PresShell::RemoveSheet(SheetType aType, StyleSheet* aSheet) NS_IMETHODIMP PresShell::SetDisplaySelection(int16_t aToggle) { - mSelection->SetDisplaySelection(aToggle); + RefPtr frameSelection = mSelection; + frameSelection->SetDisplaySelection(aToggle); return NS_OK; } NS_IMETHODIMP PresShell::GetDisplaySelection(int16_t *aToggle) { - *aToggle = mSelection->GetDisplaySelection(); + RefPtr frameSelection = mSelection; + *aToggle = frameSelection->GetDisplaySelection(); return NS_OK; } @@ -1599,8 +1603,9 @@ PresShell::GetSelection(RawSelectionType aRawSelectionType, if (!aSelection || !mSelection) return NS_ERROR_NULL_POINTER; + RefPtr frameSelection = mSelection; nsCOMPtr selection = - mSelection->GetSelection(ToSelectionType(aRawSelectionType)); + frameSelection->GetSelection(ToSelectionType(aRawSelectionType)); if (!selection) { return NS_ERROR_INVALID_ARG; @@ -1616,7 +1621,8 @@ PresShell::GetCurrentSelection(SelectionType aSelectionType) if (!mSelection) return nullptr; - return mSelection->GetSelection(aSelectionType); + RefPtr frameSelection = mSelection; + return frameSelection->GetSelection(aSelectionType); } already_AddRefed @@ -1658,8 +1664,9 @@ PresShell::ScrollSelectionIntoView(RawSelectionType aRawSelectionType, if (!mSelection) return NS_ERROR_NULL_POINTER; - return mSelection->ScrollSelectionIntoView(ToSelectionType(aRawSelectionType), - aRegion, aFlags); + RefPtr frameSelection = mSelection; + return frameSelection->ScrollSelectionIntoView( + ToSelectionType(aRawSelectionType), aRegion, aFlags); } NS_IMETHODIMP @@ -1668,7 +1675,8 @@ PresShell::RepaintSelection(RawSelectionType aRawSelectionType) if (!mSelection) return NS_ERROR_NULL_POINTER; - return mSelection->RepaintSelection(ToSelectionType(aRawSelectionType)); + RefPtr frameSelection = mSelection; + return frameSelection->RepaintSelection(ToSelectionType(aRawSelectionType)); } // Make shell be a document observer @@ -2204,31 +2212,36 @@ NS_IMETHODIMP PresShell::GetSelectionFlags(int16_t *aOutEnable) NS_IMETHODIMP PresShell::PhysicalMove(int16_t aDirection, int16_t aAmount, bool aExtend) { - return mSelection->PhysicalMove(aDirection, aAmount, aExtend); + RefPtr frameSelection = mSelection; + return frameSelection->PhysicalMove(aDirection, aAmount, aExtend); } NS_IMETHODIMP PresShell::CharacterMove(bool aForward, bool aExtend) { - return mSelection->CharacterMove(aForward, aExtend); + RefPtr frameSelection = mSelection; + return frameSelection->CharacterMove(aForward, aExtend); } NS_IMETHODIMP PresShell::CharacterExtendForDelete() { - return mSelection->CharacterExtendForDelete(); + RefPtr frameSelection = mSelection; + return frameSelection->CharacterExtendForDelete(); } NS_IMETHODIMP PresShell::CharacterExtendForBackspace() { - return mSelection->CharacterExtendForBackspace(); + RefPtr frameSelection = mSelection; + return frameSelection->CharacterExtendForBackspace(); } NS_IMETHODIMP PresShell::WordMove(bool aForward, bool aExtend) { - nsresult result = mSelection->WordMove(aForward, aExtend); + RefPtr frameSelection = mSelection; + nsresult result = frameSelection->WordMove(aForward, aExtend); // if we can't go down/up any more we must then move caret completely to // end/beginning respectively. if (NS_FAILED(result)) @@ -2239,13 +2252,15 @@ PresShell::WordMove(bool aForward, bool aExtend) NS_IMETHODIMP PresShell::WordExtendForDelete(bool aForward) { - return mSelection->WordExtendForDelete(aForward); + RefPtr frameSelection = mSelection; + return frameSelection->WordExtendForDelete(aForward); } NS_IMETHODIMP PresShell::LineMove(bool aForward, bool aExtend) { - nsresult result = mSelection->LineMove(aForward, aExtend); + RefPtr frameSelection = mSelection; + nsresult result = frameSelection->LineMove(aForward, aExtend); // if we can't go down/up any more we must then move caret completely to // end/beginning respectively. if (NS_FAILED(result)) @@ -2256,7 +2271,8 @@ PresShell::LineMove(bool aForward, bool aExtend) NS_IMETHODIMP PresShell::IntraLineMove(bool aForward, bool aExtend) { - return mSelection->IntraLineMove(aForward, aExtend); + RefPtr frameSelection = mSelection; + return frameSelection->IntraLineMove(aForward, aExtend); } @@ -2269,7 +2285,8 @@ PresShell::PageMove(bool aForward, bool aExtend) if (!scrollableFrame) return NS_OK; - mSelection->CommonPageMove(aForward, aExtend, scrollableFrame); + RefPtr frameSelection = mSelection; + frameSelection->CommonPageMove(aForward, aExtend, scrollableFrame); // After ScrollSelectionIntoView(), the pending notifications might be // flushed and PresShell/PresContext/Frames may be dead. See bug 418470. return ScrollSelectionIntoView(nsISelectionController::SELECTION_NORMAL, @@ -2362,19 +2379,21 @@ PresShell::CompleteMove(bool aForward, bool aExtend) { // Beware! This may flush notifications via synchronous // ScrollSelectionIntoView. - nsIContent* limiter = mSelection->GetAncestorLimiter(); + RefPtr frameSelection = mSelection; + nsIContent* limiter = frameSelection->GetAncestorLimiter(); nsIFrame* frame = limiter ? limiter->GetPrimaryFrame() : FrameConstructor()->GetRootElementFrame(); if (!frame) return NS_ERROR_FAILURE; nsIFrame::CaretPosition pos = frame->GetExtremeCaretPosition(!aForward); - mSelection->HandleClick(pos.mResultContent, pos.mContentOffset, - pos.mContentOffset, aExtend, false, - aForward ? CARET_ASSOCIATE_AFTER : CARET_ASSOCIATE_BEFORE); + frameSelection->HandleClick(pos.mResultContent, pos.mContentOffset, + pos.mContentOffset, aExtend, false, + aForward ? CARET_ASSOCIATE_AFTER : + CARET_ASSOCIATE_BEFORE); if (limiter) { // HandleClick resets ancestorLimiter, so set it again. - mSelection->SetAncestorLimiter(limiter); + frameSelection->SetAncestorLimiter(limiter); } // After ScrollSelectionIntoView(), the pending notifications might be @@ -2388,7 +2407,8 @@ PresShell::CompleteMove(bool aForward, bool aExtend) NS_IMETHODIMP PresShell::SelectAll() { - return mSelection->SelectAll(); + RefPtr frameSelection = mSelection; + return frameSelection->SelectAll(); } static void @@ -3170,7 +3190,7 @@ PresShell::GoToAnchor(const nsAString& aAnchorName, bool aScroll, NS_ASSERTION(node, "No nsIDOMNode for descendant of anchor"); jumpToRange->SelectNodeContents(node); // Select the anchor - nsISelection* sel = mSelection->GetSelection(SelectionType::eNormal); + RefPtr sel = mSelection->GetSelection(SelectionType::eNormal); if (sel) { sel->RemoveAllRanges(); sel->AddRange(jumpToRange); diff --git a/layout/generic/nsSelection.cpp b/layout/generic/nsSelection.cpp index c1e213453933..3e8218478a00 100644 --- a/layout/generic/nsSelection.cpp +++ b/layout/generic/nsSelection.cpp @@ -316,7 +316,8 @@ public: nsPoint pt = mPoint - frame->GetOffsetTo(mPresContext->PresShell()->FrameManager()->GetRootFrame()); - mFrameSelection->HandleDrag(frame, pt); + RefPtr frameSelection = mFrameSelection; + frameSelection->HandleDrag(frame, pt); if (!frame.IsAlive()) { return NS_OK; } @@ -4794,8 +4795,10 @@ Selection::GetAncestorLimiter(nsIContent** aContent) NS_IMETHODIMP Selection::SetAncestorLimiter(nsIContent* aContent) { - if (mFrameSelection) - mFrameSelection->SetAncestorLimiter(aContent); + if (mFrameSelection) { + RefPtr frameSelection = mFrameSelection; + frameSelection->SetAncestorLimiter(aContent); + } return NS_OK; } @@ -4952,11 +4955,12 @@ Selection::RemoveAllRanges(ErrorResult& aRv) } // Turn off signal for table selection - mFrameSelection->ClearTableCellSelection(); + RefPtr frameSelection = mFrameSelection; + frameSelection->ClearTableCellSelection(); // Be aware, this instance may be destroyed after this call. // XXX Why doesn't this call Selection::NotifySelectionListener() directly? - result = mFrameSelection->NotifySelectionListeners(GetType()); + result = frameSelection->NotifySelectionListeners(GetType()); // Also need to notify the frames! // PresShell::CharacterDataChanged should do that on DocumentChanged @@ -5044,7 +5048,8 @@ Selection::AddRangeInternal(nsRange& aRange, nsIDocument* aDocument, // Be aware, this instance may be destroyed after this call. // XXX Why doesn't this call Selection::NotifySelectionListener() directly? - result = mFrameSelection->NotifySelectionListeners(GetType()); + RefPtr frameSelection = mFrameSelection; + result = frameSelection->NotifySelectionListeners(GetType()); if (NS_FAILED(result)) { aRv.Throw(result); } @@ -5141,7 +5146,8 @@ Selection::RemoveRange(nsRange& aRange, ErrorResult& aRv) // Be aware, this instance may be destroyed after this call. // XXX Why doesn't this call Selection::NotifySelectionListener() directly? - rv = mFrameSelection->NotifySelectionListeners(GetType()); + RefPtr frameSelection = mFrameSelection; + rv = frameSelection->NotifySelectionListeners(GetType()); if (NS_FAILED(rv)) { aRv.Throw(rv); } @@ -5194,8 +5200,9 @@ Selection::Collapse(nsINode& aParentNode, uint32_t aOffset, ErrorResult& aRv) nsCOMPtr parentNode = &aParentNode; - mFrameSelection->InvalidateDesiredPos(); - if (!IsValidSelectionPoint(mFrameSelection, parentNode)) { + RefPtr frameSelection = mFrameSelection; + frameSelection->InvalidateDesiredPos(); + if (!IsValidSelectionPoint(frameSelection, parentNode)) { aRv.Throw(NS_ERROR_FAILURE); return; } @@ -5211,10 +5218,10 @@ Selection::Collapse(nsINode& aParentNode, uint32_t aOffset, ErrorResult& aRv) Clear(presContext); // Turn off signal for table selection - mFrameSelection->ClearTableCellSelection(); + frameSelection->ClearTableCellSelection(); // Hack to display the caret on the right line (bug 1237236). - if (mFrameSelection->GetHint() != CARET_ASSOCIATE_AFTER && + if (frameSelection->GetHint() != CARET_ASSOCIATE_AFTER && parentNode->IsContent()) { int32_t frameOffset; nsTextFrame* f = @@ -5225,7 +5232,7 @@ Selection::Collapse(nsINode& aParentNode, uint32_t aOffset, ErrorResult& aRv) f->GetContentEnd() == int32_t(aOffset)) || (parentNode == f->GetContent()->GetParentNode() && parentNode->IndexOf(f->GetContent()) + 1 == int32_t(aOffset))) { - mFrameSelection->SetHint(CARET_ASSOCIATE_AFTER); + frameSelection->SetHint(CARET_ASSOCIATE_AFTER); } } } @@ -5262,7 +5269,7 @@ Selection::Collapse(nsINode& aParentNode, uint32_t aOffset, ErrorResult& aRv) // Be aware, this instance may be destroyed after this call. // XXX Why doesn't this call Selection::NotifySelectionListener() directly? - result = mFrameSelection->NotifySelectionListeners(GetType()); + result = frameSelection->NotifySelectionListeners(GetType()); if (NS_FAILED(result)) { aRv.Throw(result); } @@ -5845,7 +5852,8 @@ Selection::Extend(nsINode& aParentNode, uint32_t aOffset, ErrorResult& aRv) // Be aware, this instance may be destroyed after this call. // XXX Why doesn't this call Selection::NotifySelectionListener() directly? - res = mFrameSelection->NotifySelectionListeners(GetType()); + RefPtr frameSelection = mFrameSelection; + res = frameSelection->NotifySelectionListeners(GetType()); if (NS_FAILED(res)) { aRv.Throw(res); } @@ -6455,8 +6463,9 @@ Selection::NotifySelectionListeners() } } - if (mFrameSelection->GetBatching()) { - mFrameSelection->SetDirty(); + RefPtr frameSelection = mFrameSelection; + if (frameSelection->GetBatching()) { + frameSelection->SetDirty(); return NS_OK; } nsCOMArray selectionListeners(mSelectionListeners); @@ -6471,7 +6480,7 @@ Selection::NotifySelectionListeners() domdoc = do_QueryInterface(ps->GetDocument()); } - short reason = mFrameSelection->PopReason(); + short reason = frameSelection->PopReason(); for (int32_t i = 0; i < cnt; i++) { selectionListeners[i]->NotifySelectionChanged(domdoc, this, reason); } @@ -6481,9 +6490,10 @@ Selection::NotifySelectionListeners() NS_IMETHODIMP Selection::StartBatchChanges() { - if (mFrameSelection) - mFrameSelection->StartBatchChanges(); - + if (mFrameSelection) { + RefPtr frameSelection = mFrameSelection; + frameSelection->StartBatchChanges(); + } return NS_OK; } @@ -6499,7 +6509,8 @@ nsresult Selection::EndBatchChangesInternal(int16_t aReason) { if (mFrameSelection) { - mFrameSelection->EndBatchChanges(aReason); + RefPtr frameSelection = mFrameSelection; + frameSelection->EndBatchChanges(aReason); } return NS_OK; } @@ -6538,7 +6549,8 @@ Selection::DeleteFromDocument(ErrorResult& aRv) { if (!mFrameSelection) return;//nothing to do - nsresult rv = mFrameSelection->DeleteFromDocument(); + RefPtr frameSelection = mFrameSelection; + nsresult rv = frameSelection->DeleteFromDocument(); if (NS_FAILED(rv)) { aRv.Throw(rv); } @@ -6645,14 +6657,15 @@ Selection::Modify(const nsAString& aAlter, const nsAString& aDirection, // direction, but we just ignore this error unless it's a line move, in which // case we call nsISelectionController::CompleteMove to move the cursor to // the beginning/end of the line. - rv = mFrameSelection->MoveCaret(forward ? eDirNext : eDirPrevious, - extend, amount, - visual ? nsFrameSelection::eVisual - : nsFrameSelection::eLogical); + RefPtr frameSelection = mFrameSelection; + rv = frameSelection->MoveCaret(forward ? eDirNext : eDirPrevious, + extend, amount, + visual ? nsFrameSelection::eVisual + : nsFrameSelection::eLogical); if (aGranularity.LowerCaseEqualsLiteral("line") && NS_FAILED(rv)) { nsCOMPtr shell = - do_QueryInterface(mFrameSelection->GetShell()); + do_QueryInterface(frameSelection->GetShell()); if (!shell) return; shell->CompleteMove(forward, extend); @@ -6730,13 +6743,15 @@ Selection::SelectionLanguageChange(bool aLangRTL) if (!mFrameSelection) return NS_ERROR_NOT_INITIALIZED; // Can't do selection + RefPtr frameSelection = mFrameSelection; + // if the direction of the language hasn't changed, nothing to do nsBidiLevel kbdBidiLevel = aLangRTL ? NSBIDI_RTL : NSBIDI_LTR; - if (kbdBidiLevel == mFrameSelection->mKbdBidiLevel) { + if (kbdBidiLevel == frameSelection->mKbdBidiLevel) { return NS_OK; } - mFrameSelection->mKbdBidiLevel = kbdBidiLevel; + frameSelection->mKbdBidiLevel = kbdBidiLevel; nsresult result; nsIFrame *focusFrame = 0; @@ -6767,7 +6782,7 @@ Selection::SelectionLanguageChange(bool aLangRTL) // the cursor is at a frame boundary, so use GetPrevNextBidiLevels to find the level of the characters // before and after the cursor nsCOMPtr focusContent = do_QueryInterface(GetFocusNode()); - nsPrevNextBidiLevels levels = mFrameSelection-> + nsPrevNextBidiLevels levels = frameSelection-> GetPrevNextBidiLevels(focusContent, focusOffset, false); levelBefore = levels.mLevelBefore; @@ -6782,22 +6797,22 @@ Selection::SelectionLanguageChange(bool aLangRTL) if ((level != levelBefore) && (level != levelAfter)) level = std::min(levelBefore, levelAfter); if (IS_SAME_DIRECTION(level, kbdBidiLevel)) - mFrameSelection->SetCaretBidiLevel(level); + frameSelection->SetCaretBidiLevel(level); else - mFrameSelection->SetCaretBidiLevel(level + 1); + frameSelection->SetCaretBidiLevel(level + 1); } else { // if cursor is between characters with opposite orientations, changing the keyboard language must change // the cursor level to that of the adjacent character with the orientation corresponding to the new language. if (IS_SAME_DIRECTION(levelBefore, kbdBidiLevel)) - mFrameSelection->SetCaretBidiLevel(levelBefore); + frameSelection->SetCaretBidiLevel(levelBefore); else - mFrameSelection->SetCaretBidiLevel(levelAfter); + frameSelection->SetCaretBidiLevel(levelAfter); } // The caret might have moved, so invalidate the desired position // for future usages of up-arrow or down-arrow - mFrameSelection->InvalidateDesiredPos(); + frameSelection->InvalidateDesiredPos(); return NS_OK; } @@ -7053,7 +7068,7 @@ SelectionChangeListener::NotifySelectionChanged(nsIDOMDocument* aDoc, // anonymous subtree of the node we want to fire the event on. We need to // climb up the parent chain to escape the native anonymous subtree, and then // fire the event. - if (nsFrameSelection* fs = sel->GetFrameSelection()) { + if (const nsFrameSelection* fs = sel->GetFrameSelection()) { if (nsCOMPtr root = fs->GetLimiter()) { while (root && root->IsInNativeAnonymousSubtree()) { root = root->GetParent(); @@ -7075,7 +7090,7 @@ SelectionChangeListener::NotifySelectionChanged(nsIDOMDocument* aDoc, asyncDispatcher->PostDOMEvent(); } } else { - if (nsFrameSelection* fs = sel->GetFrameSelection()) { + if (const nsFrameSelection* fs = sel->GetFrameSelection()) { if (nsCOMPtr root = fs->GetLimiter()) { if (root->IsInNativeAnonymousSubtree()) { return NS_OK;