Backed out changeset 10e71da98b14 (bug 1246918)

layout/base/AccessibleCaret.h

@@ -199,9 +199,9 @@ class AccessibleCaret
 
   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;
 

layout/base/AccessibleCaretEventHub.cpp

@@ -459,7 +459,7 @@ AccessibleCaretEventHub::Terminate()
     mScrollEndInjectorTimer->Cancel();
   }
 
-  mManager->Terminate();
+  mManager = nullptr;
   mPresShell = nullptr;
   mInitialized = false;
 }

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<nsIPresShell> presShell = mPresShell;
 
   FlushLayout();
-  if (IsTerminated()) {
+  if (presShell->IsDestroying()) {
     return;
   }
 

layout/base/AccessibleCaretManager.h

@@ -50,9 +50,6 @@ class AccessibleCaretManager
   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 @@ class AccessibleCaretManager
   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 @@ class AccessibleCaretManager
   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<nsFrameSelection> GetFrameSelection() const;
@@ -193,9 +184,6 @@ class AccessibleCaretManager
   // ---------------------------------------------------------------------------
   // 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,21 +202,19 @@ class AccessibleCaretManager
 
   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;
 
   // ---------------------------------------------------------------------------
   // Member variables
   //
   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.

layout/base/gtest/TestAccessibleCaretManager.cpp

@@ -93,8 +93,6 @@ class AccessibleCaretManagerTester : public ::testing::Test
 
     virtual void UpdateCaretsForTilt() override {}
 
-    virtual bool IsTerminated() const override { return false; }
-
     MOCK_CONST_METHOD0(GetCaretMode, CaretMode());
     MOCK_CONST_METHOD1(DispatchCaretStateChangedEvent,
                        void(CaretChangedReason aReason));