From 8cbc76d2fd5857baf3dbe555fdf301d8c14c9f68 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 17 Feb 2016 14:34:49 +0100 Subject: [PATCH] Backed out changeset 10e71da98b14 (bug 1246918) --- layout/base/AccessibleCaret.h | 4 +-- layout/base/AccessibleCaretEventHub.cpp | 2 +- layout/base/AccessibleCaretManager.cpp | 33 ++++++------------- layout/base/AccessibleCaretManager.h | 28 ++++------------ .../base/gtest/TestAccessibleCaretManager.cpp | 2 -- 5 files changed, 20 insertions(+), 49 deletions(-) diff --git a/layout/base/AccessibleCaret.h b/layout/base/AccessibleCaret.h index b94b45900628..8ff1717e9d57 100644 --- a/layout/base/AccessibleCaret.h +++ b/layout/base/AccessibleCaret.h @@ -199,9 +199,9 @@ protected: bool mSelectionBarEnabled = false; - // AccessibleCaretManager owns us by a UniquePtr. When it's terminated by + // AccessibleCaretManager owns us. When it's destroyed by // AccessibleCaretEventHub::Terminate() which is called in - // PresShell::Destroy(), it frees us automatically. No need to worry if we + // PresShell::Destroy(), it frees us automatically. No need to worry we // outlive mPresShell. nsIPresShell* MOZ_NON_OWNING_REF const mPresShell = nullptr; diff --git a/layout/base/AccessibleCaretEventHub.cpp b/layout/base/AccessibleCaretEventHub.cpp index 4dbd1556d27a..e4a779711881 100644 --- a/layout/base/AccessibleCaretEventHub.cpp +++ b/layout/base/AccessibleCaretEventHub.cpp @@ -459,7 +459,7 @@ AccessibleCaretEventHub::Terminate() mScrollEndInjectorTimer->Cancel(); } - mManager->Terminate(); + mManager = nullptr; mPresShell = nullptr; mInitialized = false; } diff --git a/layout/base/AccessibleCaretManager.cpp b/layout/base/AccessibleCaretManager.cpp index 800e4e1aa60a..debfd03f04c0 100644 --- a/layout/base/AccessibleCaretManager.cpp +++ b/layout/base/AccessibleCaretManager.cpp @@ -104,18 +104,8 @@ AccessibleCaretManager::AccessibleCaretManager(nsIPresShell* aPresShell) } AccessibleCaretManager::~AccessibleCaretManager() -{ -} - -void -AccessibleCaretManager::Terminate() { CancelCaretTimeoutTimer(); - mCaretTimeoutTimer = nullptr; - mFirstCaret = nullptr; - mSecondCaret = nullptr; - mActiveCaret = nullptr; - mPresShell = nullptr; } nsresult @@ -142,6 +132,7 @@ AccessibleCaretManager::OnSelectionChanged(nsIDOMDocument* aDoc, // Update visible carets, if javascript changes are allowed. if (sCaretsScriptUpdates && (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible())) { + FlushLayout(); UpdateCarets(); return NS_OK; } @@ -201,11 +192,6 @@ AccessibleCaretManager::DoNotShowCarets() void AccessibleCaretManager::UpdateCarets(UpdateCaretsHint aHint) { - FlushLayout(); - if (IsTerminated()) { - return; - } - mLastUpdateCaretMode = GetCaretMode(); switch (mLastUpdateCaretMode) { @@ -386,9 +372,6 @@ AccessibleCaretManager::UpdateCaretsForSelectionMode(UpdateCaretsHint aHint) secondCaretResult == PositionChangedResult::Changed) { // Flush layout to make the carets intersection correct. FlushLayout(); - if (IsTerminated()) { - return; - } } if (aHint == UpdateCaretsHint::Default) { @@ -611,6 +594,11 @@ AccessibleCaretManager::OnScrollEnd() mFirstCaret->SetAppearance(mFirstCaretAppearanceOnScrollStart); mSecondCaret->SetAppearance(mSecondCaretAppearanceOnScrollStart); + // Flush layout to make the carets intersection correct since we turn the + // appearance of the carets from None or NormalNotShown into something + // visible. + FlushLayout(); + if (GetCaretMode() == CaretMode::Cursor) { if (!mFirstCaret->IsLogicallyVisible()) { // If the caret is hidden (Appearance::None) due to timeout or blur, no @@ -1144,7 +1132,7 @@ AccessibleCaretManager::CaretTimeoutMs() const void AccessibleCaretManager::LaunchCaretTimeoutTimer() { - if (!mPresShell || !mCaretTimeoutTimer || CaretTimeoutMs() == 0 || + if (!mCaretTimeoutTimer || CaretTimeoutMs() == 0 || GetCaretMode() != CaretMode::Cursor || mActiveCaret) { return; } @@ -1171,12 +1159,11 @@ AccessibleCaretManager::CancelCaretTimeoutTimer() void AccessibleCaretManager::DispatchCaretStateChangedEvent(CaretChangedReason aReason) const { - if (!mPresShell) { - return; - } + // Holding PresShell to prevent AccessibleCaretManager to be destroyed. + nsCOMPtr presShell = mPresShell; FlushLayout(); - if (IsTerminated()) { + if (presShell->IsDestroying()) { return; } diff --git a/layout/base/AccessibleCaretManager.h b/layout/base/AccessibleCaretManager.h index c1da5c9bbdc9..4989d4202a00 100644 --- a/layout/base/AccessibleCaretManager.h +++ b/layout/base/AccessibleCaretManager.h @@ -50,9 +50,6 @@ public: explicit AccessibleCaretManager(nsIPresShell* aPresShell); virtual ~AccessibleCaretManager(); - // Called by AccessibleCaretEventHub to inform us that PresShell is destroyed. - void Terminate(); - // The aPoint in the following public methods should be relative to root // frame. @@ -127,9 +124,7 @@ protected: friend std::ostream& operator<<(std::ostream& aStream, const UpdateCaretsHint& aResult); - // Update carets based on current selection status. This function will flush - // layout, so caller must ensure the PresShell is still valid after calling - // this method. + // Update carets based on current selection status. void UpdateCarets(UpdateCaretsHint aHint = UpdateCaretsHint::Default); // Force hiding all carets regardless of the current selection status. @@ -165,11 +160,7 @@ protected: nsresult DragCaretInternal(const nsPoint& aPoint); nsPoint AdjustDragBoundary(const nsPoint& aPoint) const; void ClearMaintainedSelection() const; - - // Caller is responsible to use IsTerminated() to check whether PresShell is - // still valid. void FlushLayout() const; - dom::Element* GetEditingHostForFrame(nsIFrame* aFrame) const; dom::Selection* GetSelection() const; already_AddRefed GetFrameSelection() const; @@ -193,9 +184,6 @@ protected: // --------------------------------------------------------------------------- // The following functions are made virtual for stubbing or mocking in gtest. // - // @return true if Terminate() had been called. - virtual bool IsTerminated() const { return !mPresShell; } - // Get caret mode based on current selection. virtual CaretMode GetCaretMode() const; @@ -214,8 +202,8 @@ protected: virtual bool HasNonEmptyTextContent(nsINode* aNode) const; - // This function will flush layout, so caller must ensure the PresShell is - // still valid after calling this method. + // This function will call FlushPendingNotifications. So caller must ensure + // everything exists after calling this method. virtual void DispatchCaretStateChangedEvent(dom::CaretChangedReason aReason) const; // --------------------------------------------------------------------------- @@ -223,12 +211,10 @@ protected: // nscoord mOffsetYToCaretLogicalPosition = NS_UNCONSTRAINEDSIZE; - // AccessibleCaretEventHub owns us by a UniquePtr. When it's destroyed, we'll - // also be destroyed. No need to worry if we outlive mPresShell. - // - // mPresShell will be set to nullptr in Terminate(). Therefore mPresShell is - // nullptr either we are in gtest or PresShell::IsDestroying() is true. - nsIPresShell* MOZ_NON_OWNING_REF mPresShell = nullptr; + // AccessibleCaretEventHub owns us. When it's Terminate() called by + // PresShell::Destroy(), we will be destroyed. No need to worry we outlive + // mPresShell. + nsIPresShell* MOZ_NON_OWNING_REF const mPresShell = nullptr; // First caret is attached to nsCaret in cursor mode, and is attached to // selection highlight as the left caret in selection mode. diff --git a/layout/base/gtest/TestAccessibleCaretManager.cpp b/layout/base/gtest/TestAccessibleCaretManager.cpp index e8b97a382feb..ef2b905f1cec 100644 --- a/layout/base/gtest/TestAccessibleCaretManager.cpp +++ b/layout/base/gtest/TestAccessibleCaretManager.cpp @@ -93,8 +93,6 @@ public: virtual void UpdateCaretsForTilt() override {} - virtual bool IsTerminated() const override { return false; } - MOCK_CONST_METHOD0(GetCaretMode, CaretMode()); MOCK_CONST_METHOD1(DispatchCaretStateChangedEvent, void(CaretChangedReason aReason));