Bug 1285273 - Bail out early if ptFrame died in SelectWordOrShortcut(). r=masayuki
authorTing-Yu Lin <aethanyc@gmail.com>
Sun, 10 Jul 2016 14:36:02 +0800
changeset 329725 23da74f8393fc329339e400c242e7309c81cbeb2
parent 329724 4f7bcfbda34b2c5f4b042abc296a132dc3f53903
child 329726 621f32c868bb5654a1ae2dc25cbb622b3c0efd69
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1285273
milestone50.0a1
Bug 1285273 - Bail out early if ptFrame died in SelectWordOrShortcut(). r=masayuki Check ptFrame is still alive after calling ChangeFocusToOrClearOldFocus() and IMEStateManager::NotifyIME(). MozReview-Commit-ID: DtjoxtRIDdK
layout/base/AccessibleCaretManager.cpp
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -533,31 +533,39 @@ AccessibleCaretManager::SelectWordOrShor
   }
 
   nsIFrame* rootFrame = mPresShell->GetRootFrame();
   if (!rootFrame) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // Find the frame under point.
-  nsIFrame* ptFrame = nsLayoutUtils::GetFrameForPoint(rootFrame, aPoint,
+  nsWeakFrame ptFrame = nsLayoutUtils::GetFrameForPoint(rootFrame, aPoint,
     nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | nsLayoutUtils::IGNORE_CROSS_DOC);
-  if (!ptFrame) {
+  if (!ptFrame.IsAlive()) {
     return NS_ERROR_FAILURE;
   }
 
   nsIFrame* focusableFrame = GetFocusableFrame(ptFrame);
 
 #ifdef DEBUG_FRAME_DUMP
   AC_LOG("%s: Found %s under (%d, %d)", __FUNCTION__, ptFrame->ListTag().get(),
          aPoint.x, aPoint.y);
   AC_LOG("%s: Found %s focusable", __FUNCTION__,
          focusableFrame ? focusableFrame->ListTag().get() : "no frame");
 #endif
 
+  // Get ptInFrame here so that we don't need to check whether rootFrame is
+  // alive later. Note that if ptFrame is being moved by
+  // IMEStateManager::NotifyIME() or ChangeFocusToOrClearOldFocus() below,
+  // something under the original point will be selected, which may not be the
+  // original text the user wants to select.
+  nsPoint ptInFrame = aPoint;
+  nsLayoutUtils::TransformPoint(rootFrame, ptFrame, ptInFrame);
+
   // Firstly check long press on an empty editable content.
   Element* newFocusEditingHost = GetEditingHostForFrame(ptFrame);
   if (focusableFrame && newFocusEditingHost &&
       !HasNonEmptyTextContent(newFocusEditingHost)) {
     ChangeFocusToOrClearOldFocus(focusableFrame);
 
     if (sCaretShownWhenLongTappingOnEmptyContent) {
       mFirstCaret->SetAppearance(Appearance::Normal);
@@ -580,34 +588,39 @@ AccessibleCaretManager::SelectWordOrShor
   if (!selectable) {
     return NS_ERROR_FAILURE;
   }
 
   // Commit the composition string of the old editable focus element (if there
   // is any) before changing the focus.
   IMEStateManager::NotifyIME(widget::REQUEST_TO_COMMIT_COMPOSITION,
                              mPresShell->GetPresContext());
+  if (!ptFrame.IsAlive()) {
+    // Cannot continue because ptFrame died.
+    return NS_ERROR_FAILURE;
+  }
 
   // ptFrame is selectable. Now change the focus.
   ChangeFocusToOrClearOldFocus(focusableFrame);
+  if (!ptFrame.IsAlive()) {
+    // Cannot continue because ptFrame died.
+    return NS_ERROR_FAILURE;
+  }
 
   if (GetCaretMode() == CaretMode::Selection &&
       !mFirstCaret->IsLogicallyVisible() && !mSecondCaret->IsLogicallyVisible()) {
     // We have a selection while both carets have Appearance::None because of
     // previous operations like blur event. Just update carets on the selection
     // without selecting a new word.
     AC_LOG("%s: UpdateCarets() for current selection", __FUNCTION__);
     UpdateCaretsWithHapticFeedback();
     return NS_OK;
   }
 
   // Then try select a word under point.
-  nsPoint ptInFrame = aPoint;
-  nsLayoutUtils::TransformPoint(rootFrame, ptFrame, ptInFrame);
-
   nsresult rv = SelectWord(ptFrame, ptInFrame);
   UpdateCaretsWithHapticFeedback();
 
   return rv;
 }
 
 void
 AccessibleCaretManager::OnScrollStart()