Backed out changeset 10e71da98b14 (bug 1246918)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Wed, 17 Feb 2016 14:34:49 +0100
changeset 284542 7ced13d644be8b05f0de4beeaaa3458910eb2ad6
parent 284536 c007ec81b75ae1c51da2c2d2bab21180290fab6b
child 284543 709f559b5406e8555cf84dd09bdc747b076f142c
push id71996
push usercbook@mozilla.com
push dateWed, 17 Feb 2016 13:37:27 +0000
treeherdermozilla-inbound@428c9570954f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1246918
milestone47.0a1
backs out10e71da98b144fbf42aaa81a1056910a3766a6cb
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 10e71da98b14 (bug 1246918)
layout/base/AccessibleCaret.h
layout/base/AccessibleCaretEventHub.cpp
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
layout/base/gtest/TestAccessibleCaretManager.cpp
--- a/layout/base/AccessibleCaret.h
+++ b/layout/base/AccessibleCaret.h
@@ -194,19 +194,19 @@ protected:
     virtual ~DummyTouchListener() {};
   };
 
   // Member variables
   Appearance mAppearance = Appearance::None;
 
   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;
 
   RefPtr<dom::AnonymousContent> mCaretElementHolder;
 
   // mImaginaryCaretRect is relative to root frame.
   nsRect mImaginaryCaretRect;
 
--- a/layout/base/AccessibleCaretEventHub.cpp
+++ b/layout/base/AccessibleCaretEventHub.cpp
@@ -454,17 +454,17 @@ AccessibleCaretEventHub::Terminate()
   if (mLongTapInjectorTimer) {
     mLongTapInjectorTimer->Cancel();
   }
 
   if (mScrollEndInjectorTimer) {
     mScrollEndInjectorTimer->Cancel();
   }
 
-  mManager->Terminate();
+  mManager = nullptr;
   mPresShell = nullptr;
   mInitialized = false;
 }
 
 nsEventStatus
 AccessibleCaretEventHub::HandleEvent(WidgetEvent* aEvent)
 {
   nsEventStatus status = nsEventStatus_eIgnore;
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -100,27 +100,17 @@ AccessibleCaretManager::AccessibleCaretM
     Preferences::AddBoolVarCache(&sHapticFeedback,
                                  "layout.accessiblecaret.hapticfeedback");
     addedPrefs = true;
   }
 }
 
 AccessibleCaretManager::~AccessibleCaretManager()
 {
-}
-
-void
-AccessibleCaretManager::Terminate()
-{
   CancelCaretTimeoutTimer();
-  mCaretTimeoutTimer = nullptr;
-  mFirstCaret = nullptr;
-  mSecondCaret = nullptr;
-  mActiveCaret = nullptr;
-  mPresShell = nullptr;
 }
 
 nsresult
 AccessibleCaretManager::OnSelectionChanged(nsIDOMDocument* aDoc,
                                            nsISelection* aSel, int16_t aReason)
 {
   Selection* selection = GetSelection();
   AC_LOG("%s: aSel: %p, GetSelection(): %p, aReason: %d", __FUNCTION__,
@@ -137,16 +127,17 @@ AccessibleCaretManager::OnSelectionChang
     return NS_OK;
   }
 
   // Move the cursor by Javascript / or unknown internal.
   if (aReason == nsISelectionListener::NO_REASON) {
     // Update visible carets, if javascript changes are allowed.
     if (sCaretsScriptUpdates &&
         (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible())) {
+        FlushLayout();
         UpdateCarets();
         return NS_OK;
     }
     // Default for NO_REASON is to make hidden.
     HideCarets();
     return NS_OK;
   }
 
@@ -196,21 +187,16 @@ AccessibleCaretManager::DoNotShowCarets(
     DispatchCaretStateChangedEvent(CaretChangedReason::Visibilitychange);
     CancelCaretTimeoutTimer();
   }
 }
 
 void
 AccessibleCaretManager::UpdateCarets(UpdateCaretsHint aHint)
 {
-  FlushLayout();
-  if (IsTerminated()) {
-    return;
-  }
-
   mLastUpdateCaretMode = GetCaretMode();
 
   switch (mLastUpdateCaretMode) {
   case CaretMode::None:
     HideCarets();
     break;
   case CaretMode::Cursor:
     UpdateCaretsForCursorMode(aHint);
@@ -381,19 +367,16 @@ AccessibleCaretManager::UpdateCaretsForS
     updateSingleCaret(mFirstCaret.get(), startFrame, startOffset);
   PositionChangedResult secondCaretResult =
     updateSingleCaret(mSecondCaret.get(), endFrame, endOffset);
 
   if (firstCaretResult == PositionChangedResult::Changed ||
       secondCaretResult == PositionChangedResult::Changed) {
     // Flush layout to make the carets intersection correct.
     FlushLayout();
-    if (IsTerminated()) {
-      return;
-    }
   }
 
   if (aHint == UpdateCaretsHint::Default) {
     // Only check for tilt carets with default update hint. Otherwise we might
     // override the appearance set by the caller.
     UpdateCaretsForTilt();
   }
 
@@ -606,16 +589,21 @@ AccessibleCaretManager::OnScrollEnd()
 {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
   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
       // need to update it.
       return;
     }
   }
 
@@ -1139,17 +1127,17 @@ AccessibleCaretManager::CaretTimeoutMs()
   }
 
   return caretTimeoutMs;
 }
 
 void
 AccessibleCaretManager::LaunchCaretTimeoutTimer()
 {
-  if (!mPresShell || !mCaretTimeoutTimer || CaretTimeoutMs() == 0 ||
+  if (!mCaretTimeoutTimer || CaretTimeoutMs() == 0 ||
       GetCaretMode() != CaretMode::Cursor || mActiveCaret) {
     return;
   }
 
   nsTimerCallbackFunc callback = [](nsITimer* aTimer, void* aClosure) {
     auto self = static_cast<AccessibleCaretManager*>(aClosure);
     if (self->GetCaretMode() == CaretMode::Cursor) {
       self->HideCarets();
@@ -1166,22 +1154,21 @@ AccessibleCaretManager::CancelCaretTimeo
   if (mCaretTimeoutTimer) {
     mCaretTimeoutTimer->Cancel();
   }
 }
 
 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;
   }
 
   Selection* sel = GetSelection();
   if (!sel) {
     return;
   }
 
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -45,19 +45,16 @@ class Selection;
 // https://wiki.mozilla.org/Copy_n_Paste
 //
 class AccessibleCaretManager
 {
 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.
 
   // Press caret on the given point. Return NS_OK if the point is actually on
   // one of the carets.
   virtual nsresult PressCaret(const nsPoint& aPoint);
 
   // Drag caret to the given point. It's required to call PressCaret()
@@ -122,19 +119,17 @@ protected:
     // the caret in cursor mode is hidden due to timeout, do not change its
     // appearance to Normal.
     RespectOldAppearance
   };
 
   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.
   void HideCarets();
 
   // Force carets to be "present" logically, but not visible. Allows ActionBar
   // to stay open when carets visibility is supressed during scroll.
   void DoNotShowCarets();
@@ -160,21 +155,17 @@ protected:
   // If aBackward is false, find the first node from the first range in current
   // selection, and return the frame and the offset into that frame. If aBackward
   // is true, find the last node from the last range instead.
   nsIFrame* FindFirstNodeWithFrame(bool aBackward, int32_t* aOutOffset) const;
 
   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;
 
   // Get the bounding rectangle for aFrame where the caret under cursor mode can
   // be positioned. The rectangle is relative to the root frame.
   nsRect GetContentBoundaryForFrame(nsIFrame* aFrame) const;
 
@@ -188,19 +179,16 @@ protected:
   // no one touches it.
   uint32_t CaretTimeoutMs() const;
   void LaunchCaretTimeoutTimer();
   void CancelCaretTimeoutTimer();
 
   // ---------------------------------------------------------------------------
   // 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;
 
   // @return true if aStartFrame comes before aEndFrame.
   virtual bool CompareTreePosition(nsIFrame* aStartFrame,
                                    nsIFrame* aEndFrame) const;
 
   // Check if the two carets is overlapping to become tilt.
@@ -209,31 +197,29 @@ protected:
   // Check whether AccessibleCaret is displayable in cursor mode or not.
   // @param aOutFrame returns frame of the cursor if it's displayable.
   // @param aOutOffset returns frame offset as well.
   virtual bool IsCaretDisplayableInCursorMode(nsIFrame** aOutFrame = nullptr,
                                               int32_t* aOutOffset = nullptr) const;
 
   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.
   UniquePtr<AccessibleCaret> mFirstCaret;
 
   // Second caret is used solely in selection mode, and is attached to selection
   // highlight as the right caret.
   UniquePtr<AccessibleCaret> mSecondCaret;
--- a/layout/base/gtest/TestAccessibleCaretManager.cpp
+++ b/layout/base/gtest/TestAccessibleCaretManager.cpp
@@ -88,18 +88,16 @@ public:
     virtual bool IsCaretDisplayableInCursorMode(
       nsIFrame** aOutFrame = nullptr, int32_t* aOutOffset = nullptr) const override
     {
       return true;
     }
 
     virtual void UpdateCaretsForTilt() override {}
 
-    virtual bool IsTerminated() const override { return false; }
-
     MOCK_CONST_METHOD0(GetCaretMode, CaretMode());
     MOCK_CONST_METHOD1(DispatchCaretStateChangedEvent,
                        void(CaretChangedReason aReason));
     MOCK_CONST_METHOD1(HasNonEmptyTextContent, bool(nsINode* aNode));
 
   }; // class MockAccessibleCaretManager
 
   using Appearance = AccessibleCaret::Appearance;