Bug 1285273 - Bail out early if ptFrame died in SelectWordOrShortcut(). r=masayuki a=gchang
authorTing-Yu Lin <aethanyc@gmail.com>
Sun, 10 Jul 2016 14:36:02 +0800
changeset 339938 709652b674013b136627e45fd90b4e9063906a19
parent 339937 76f20388d4f8e18634160d3669d26acf9a2e6b68
child 339939 e8946b3496615d88c59809528e98fbb3c9fa5790
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki, gchang
bugs1285273
milestone49.0a2
Bug 1285273 - Bail out early if ptFrame died in SelectWordOrShortcut(). r=masayuki a=gchang 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()