Bug 1486521 - Make Selection::Stringify() stop flushing frames if AccessibleCaretManager doesn't allow so. r=emilio
authorTing-Yu Lin <aethanyc@gmail.com>
Mon, 14 Jan 2019 04:58:59 +0000
changeset 453766 93c0ed8f369080be59f2dd3a705e9fd8290f7813
parent 453765 b45954a2a5b8f59dd7b22c7dc85722f414a3613b
child 453767 d352b4fe9d6808091c236afc68e6c6d242f9f17c
push id35372
push usercbrindusan@mozilla.com
push dateMon, 14 Jan 2019 21:49:33 +0000
treeherdermozilla-central@50b3268954b1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1486521
milestone66.0a1
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
Bug 1486521 - Make Selection::Stringify() stop flushing frames if AccessibleCaretManager doesn't allow so. r=emilio The added crashtest still crashes on Android verify runs (TV) for unknown reasons, so skip it. Differential Revision: https://phabricator.services.mozilla.com/D16395
dom/base/Selection.cpp
dom/base/Selection.h
layout/base/AccessibleCaretManager.cpp
layout/base/AccessibleCaretManager.h
layout/base/crashtests/1486521.html
layout/base/crashtests/crashtests.list
--- a/dom/base/Selection.cpp
+++ b/dom/base/Selection.cpp
@@ -378,27 +378,29 @@ void printRange(nsRange* aDomRange) {
   int32_t endOffset = aDomRange->EndOffset();
 
   printf("range: 0x%lx\t start: 0x%lx %ld, \t end: 0x%lx,%ld\n",
          (unsigned long)aDomRange, (unsigned long)startNode, (long)startOffset,
          (unsigned long)endNode, (long)endOffset);
 }
 #endif /* PRINT_RANGE */
 
-void Selection::Stringify(nsAString& aResult) {
-  // We need FlushType::Frames here to make sure frames have been created for
-  // the selected content.  Use mFrameSelection->GetShell() which returns
-  // null if the Selection has been disconnected (the shell is Destroyed).
-  nsCOMPtr<nsIPresShell> shell =
-      mFrameSelection ? mFrameSelection->GetShell() : nullptr;
-  if (!shell) {
-    aResult.Truncate();
-    return;
+void Selection::Stringify(nsAString& aResult, FlushFrames aFlushFrames) {
+  if (aFlushFrames == FlushFrames::Yes) {
+    // We need FlushType::Frames here to make sure frames have been created for
+    // the selected content.  Use mFrameSelection->GetShell() which returns
+    // null if the Selection has been disconnected (the shell is Destroyed).
+    nsCOMPtr<nsIPresShell> shell =
+        mFrameSelection ? mFrameSelection->GetShell() : nullptr;
+    if (!shell) {
+      aResult.Truncate();
+      return;
+    }
+    shell->FlushPendingNotifications(FlushType::Frames);
   }
-  shell->FlushPendingNotifications(FlushType::Frames);
 
   IgnoredErrorResult rv;
   ToStringWithFormat(NS_LITERAL_STRING("text/plain"),
                      nsIDocumentEncoder::SkipInvisibleContent, 0, aResult, rv);
   if (rv.Failed()) {
     aResult.Truncate();
   }
 }
--- a/dom/base/Selection.h
+++ b/dom/base/Selection.h
@@ -298,17 +298,22 @@ class Selection final : public nsSupport
   /**
    * RemoveAllRangesTemporarily() is useful if the caller will add one or more
    * ranges later.  This tries to cache a removing range if it's possible.
    * If a range is not referred by anything else this selection, the range
    * can be reused later.  Otherwise, this works as same as RemoveAllRanges().
    */
   nsresult RemoveAllRangesTemporarily();
 
-  void Stringify(nsAString& aResult);
+  /**
+   * Whether Stringify should flush layout or not.
+   */
+  enum class FlushFrames { No, Yes };
+  MOZ_CAN_RUN_SCRIPT
+  void Stringify(nsAString& aResult, FlushFrames = FlushFrames::Yes);
 
   /**
    * Indicates whether the node is part of the selection. If partlyContained
    * is true, the function returns true when some part of the node
    * is part of the selection. If partlyContained is false, the
    * function only returns true when the entire node is part of the selection.
    */
   bool ContainsNode(nsINode& aNode, bool aPartlyContained,
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -714,19 +714,21 @@ already_AddRefed<nsFrameSelection> Acces
     return nullptr;
   }
 
   return fs.forget();
 }
 
 nsAutoString AccessibleCaretManager::StringifiedSelection() const {
   nsAutoString str;
-  Selection* selection = GetSelection();
+  RefPtr<Selection> selection = GetSelection();
   if (selection) {
-    selection->Stringify(str);
+    selection->Stringify(str, mAllowFlushingLayout
+                                  ? Selection::FlushFrames::Yes
+                                  : Selection::FlushFrames::No);
   }
   return str;
 }
 
 Element* AccessibleCaretManager::GetEditingHostForFrame(
     nsIFrame* aFrame) const {
   if (!aFrame) {
     return nullptr;
@@ -1332,17 +1334,17 @@ void AccessibleCaretManager::DispatchCar
 
   init.mBoundingClientRect = domRect;
   init.mReason = aReason;
   init.mCollapsed = sel->IsCollapsed();
   init.mCaretVisible =
       mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible();
   init.mCaretVisuallyVisible =
       mFirstCaret->IsVisuallyVisible() || mSecondCaret->IsVisuallyVisible();
-  sel->Stringify(init.mSelectedTextContent);
+  init.mSelectedTextContent = StringifiedSelection();
 
   RefPtr<CaretStateChangedEvent> event = CaretStateChangedEvent::Constructor(
       doc, NS_LITERAL_STRING("mozcaretstatechanged"), init);
 
   event->SetTrusted(true);
   event->WidgetEventPtr()->mFlags.mOnlyChromeDispatch = true;
 
   AC_LOG("%s: reason %" PRIu32 ", collapsed %d, caretVisible %" PRIu32,
--- a/layout/base/AccessibleCaretManager.h
+++ b/layout/base/AccessibleCaretManager.h
@@ -176,27 +176,30 @@ class AccessibleCaretManager {
   // Get the nearest enclosing focusable frame of aFrame.
   // @return focusable frame if there is any; nullptr otherwise.
   nsIFrame* GetFocusableFrame(nsIFrame* aFrame) const;
 
   // Change focus to aFrame if it isn't nullptr. Otherwise, clear the old focus
   // then re-focus the window.
   void ChangeFocusToOrClearOldFocus(nsIFrame* aFrame) const;
 
+  MOZ_CAN_RUN_SCRIPT
   nsresult SelectWord(nsIFrame* aFrame, const nsPoint& aPoint) const;
   void SetSelectionDragState(bool aState) const;
 
   // Return true if the candidate string is a phone number.
   bool IsPhoneNumber(nsAString& aCandidate) const;
 
   // Extend the current selection forwards and backwards if it's already a
   // phone number.
+  MOZ_CAN_RUN_SCRIPT
   void SelectMoreIfPhoneNumber() const;
 
   // Extend the current phone number selection in the requested direction.
+  MOZ_CAN_RUN_SCRIPT
   void ExtendPhoneNumberSelection(const nsAString& aDirection) const;
 
   void SetSelectionDirection(nsDirection aDir) const;
 
   // If aDirection is eDirNext, get the frame for the range start in the first
   // range from the current selection, and return the offset into that frame as
   // well as the range start content and the content offset. Otherwise, get the
   // frame and the offset for the range end in the last range instead.
@@ -221,16 +224,18 @@ class AccessibleCaretManager {
   // See the mRefCnt assertions in AccessibleCaretEventHub.
   //
   // Returns whether mPresShell we're holding is still valid.
   MOZ_MUST_USE MOZ_CAN_RUN_SCRIPT bool FlushLayout();
 
   dom::Element* GetEditingHostForFrame(nsIFrame* aFrame) const;
   dom::Selection* GetSelection() const;
   already_AddRefed<nsFrameSelection> GetFrameSelection() const;
+
+  MOZ_CAN_RUN_SCRIPT
   nsAutoString StringifiedSelection() const;
 
   // Get the union of all the child frame scrollable overflow rects for aFrame,
   // which is used as a helper function to restrict the area where the caret can
   // be dragged. Returns the rect relative to aFrame.
   nsRect GetAllChildFrameRectsUnion(nsIFrame* aFrame) const;
 
   // Restrict the active caret's dragging position based on
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1486521.html
@@ -0,0 +1,11 @@
+<script>
+window.onload=function() {
+  a.options[10] = b;
+  document.execCommand("selectAll", false);
+  document.activeElement.hidden = true;
+  a.options.selectedIndex = 1;
+}
+</script>
+<input contenteditable>
+<select id="a" multiple>
+<option id="b" selected>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -538,16 +538,17 @@ HTTP load 1464641.html
 load 1464737.html
 load 1466638.html
 load 1467688.html
 load 1467964.html
 load 1469354.html
 pref(layout.accessiblecaret.enabled,true) load 1472020.html
 load 1472027.html
 load 1477847.html
+skip-if(verify&&Android) pref(layout.accessiblecaret.enabled,true) load 1486521.html
 load 1489149.html
 load 1490037.html
 load 1494332.html
 load 1494030.html
 load 1505420.html
 pref(layout.css.column-span.enabled,true) load 1506163.html
 pref(layout.css.column-span.enabled,true) load 1506204.html
 pref(layout.css.column-span.enabled,true) load 1506314.html