Bug 1302470 Part 2: Use a hit-test method to determine if the rect of a range is visible on the page or not to the eye, for use in find-in-page. r=mstange,smaug
☠☠ backed out by f0444d406516 ☠ ☠
authorMike de Boer <mdeboer@mozilla.com>
Wed, 09 Nov 2016 15:09:36 -0800
changeset 368363 cc04b61c9f23a5a1fc35287998df38048beef247
parent 368362 e7c190a25bfa53898f36fd9f4349c7b79942ec3a
child 368364 3ce8e7ccf045dafe326157cbd29af2eeebf5518e
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange, smaug
bugs1302470
milestone53.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 1302470 Part 2: Use a hit-test method to determine if the rect of a range is visible on the page or not to the eye, for use in find-in-page. r=mstange,smaug MozReview-Commit-ID: 9P7gf0GcREv
layout/base/nsLayoutUtils.h
toolkit/components/typeaheadfind/nsITypeAheadFind.idl
toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
toolkit/components/typeaheadfind/nsTypeAheadFind.h
toolkit/modules/FinderIterator.jsm
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -816,17 +816,17 @@ public:
   static nsIFrame* GetFrameForPoint(nsIFrame* aFrame, nsPoint aPt,
                                     uint32_t aFlags = 0);
 
   /**
    * Given aFrame, the root frame of a stacking context, find all descendant
    * frames under the area of a rectangle that receives a mouse event,
    * or nullptr if there is no such frame.
    * @param aRect the rect, relative to the frame origin
-   * @param aOutFrames an array to add all the frames found
+   * @param aOutFrames an array to append all the frames found
    * @param aFlags some combination of FrameForPointFlags
    */
   static nsresult GetFramesForArea(nsIFrame* aFrame, const nsRect& aRect,
                                    nsTArray<nsIFrame*> &aOutFrames,
                                    uint32_t aFlags = 0);
 
   /**
    * Transform aRect relative to aFrame up to the coordinate system of
--- a/toolkit/components/typeaheadfind/nsITypeAheadFind.idl
+++ b/toolkit/components/typeaheadfind/nsITypeAheadFind.idl
@@ -13,17 +13,17 @@
 /******************************** Declarations *******************************/
 
 interface mozIDOMWindow;
 interface nsIDocShell;
 
 
 /****************************** nsTypeAheadFind ******************************/
 
-[scriptable, uuid(ae501e28-c57f-4692-ac74-410e1bed98b7)]
+[scriptable, uuid(3cfe7906-f189-45a0-8abe-8e4437a23cae)]
 interface nsITypeAheadFind : nsISupports
 {
   /****************************** Initializer ******************************/
 
   /* Necessary initialization that can't happen in the constructor, either
    * because function calls here may fail, or because the docShell is
    * required. */
   void init(in nsIDocShell aDocShell);
@@ -53,17 +53,17 @@ interface nsITypeAheadFind : nsISupports
   void setSelectionModeAndRepaint(in short toggle);
 
   /* Collapse the "found match" selection to its start.  Because not all
    * matches are owned by the same selection controller, this doesn't
    * necessarily happen automatically. */
   void collapseSelection();
 
   /* Check if a range is visible */
-  boolean isRangeVisible(in nsIDOMRange aRange, in boolean aMustBeInViewPort);
+  boolean isRangeVisible(in nsIDOMRange aRange, in boolean aFlushLayout);
 
   /******************************* Attributes ******************************/
 
   readonly attribute AString searchString;
                                         // Most recent search string
   attribute boolean caseSensitive;      // Searches are case sensitive
   attribute boolean entireWord;         // Search for whole words only
   readonly attribute nsIDOMElement foundLink;
--- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
+++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -47,16 +47,17 @@
 #include "nsNameSpaceManager.h"
 #include "nsIWindowWatcher.h"
 #include "nsIObserverService.h"
 #include "nsFocusManager.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Link.h"
 #include "nsRange.h"
 #include "nsXBLBinding.h"
+#include "nsLayoutUtils.h"
 
 #include "nsTypeAheadFind.h"
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsTypeAheadFind)
   NS_INTERFACE_MAP_ENTRY(nsITypeAheadFind)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsITypeAheadFind)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
   NS_INTERFACE_MAP_ENTRY(nsIObserver)
@@ -65,18 +66,16 @@ NS_INTERFACE_MAP_END
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsTypeAheadFind)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsTypeAheadFind)
 
 NS_IMPL_CYCLE_COLLECTION(nsTypeAheadFind, mFoundLink, mFoundEditable,
                          mCurrentWindow, mStartFindRange, mSearchRange,
                          mStartPointRange, mEndPointRange, mSoundInterface,
                          mFind, mFoundRange)
 
-static NS_DEFINE_CID(kFrameTraversalCID, NS_FRAMETRAVERSAL_CID);
-
 #define NS_FIND_CONTRACTID "@mozilla.org/embedcomp/rangefind;1"
 
 nsTypeAheadFind::nsTypeAheadFind():
   mStartLinksOnlyPref(false),
   mCaretBrowsingOn(false),
   mDidAddObservers(false),
   mLastFindLength(0),
   mIsSoundInitialized(false),
@@ -431,18 +430,17 @@ nsTypeAheadFind::FindItNow(nsIPresShell 
 
       if (aIsLinksOnly) {
         // Don't check if inside link when searching all text
         RangeStartsInsideLink(returnRange, presShell, &isInsideLink,
                               &isStartingLink);
       }
 
       bool usesIndependentSelection;
-      if (!IsRangeVisible(presShell, presContext, returnRange,
-                          aIsFirstVisiblePreferred, false,
+      if (!IsRangeVisible(presShell, presContext, returnRange, true,
                           getter_AddRefs(mStartPointRange), 
                           &usesIndependentSelection) ||
           (aIsLinksOnly && !isInsideLink) ||
           (mStartLinksOnlyPref && aIsLinksOnly && !isStartingLink)) {
         // ------ Failure ------
         // At this point mStartPointRange got updated to the first
         // visible range in the viewport.  We _may_ be able to just
         // start there, if it's not taking us in the wrong direction.
@@ -812,18 +810,17 @@ nsTypeAheadFind::GetSearchContainers(nsI
     if (selection)
       selection->GetRangeAt(0, getter_AddRefs(currentSelectionRange));
   }
 
   if (!currentSelectionRange) {
     // Ensure visible range, move forward if necessary
     // This uses ignores the return value, but usese the side effect of
     // IsRangeVisible. It returns the first visible range after searchRange
-    IsRangeVisible(presShell, presContext, mSearchRange, 
-                   aIsFirstVisiblePreferred, true, 
+    IsRangeVisible(presShell, presContext, mSearchRange, true,
                    getter_AddRefs(mStartPointRange), nullptr);
   }
   else {
     int32_t startOffset;
     nsCOMPtr<nsIDOMNode> startNode;
     if (aFindPrev) {
       currentSelectionRange->GetStartContainer(getter_AddRefs(startNode));
       currentSelectionRange->GetStartOffset(&startOffset);
@@ -1156,160 +1153,96 @@ nsTypeAheadFind::GetFoundRange(nsIDOMRan
   }
 
   mFoundRange->CloneRange(aFoundRange);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsTypeAheadFind::IsRangeVisible(nsIDOMRange *aRange,
-                                bool aMustBeInViewPort,
+                                bool aFlushLayout,
                                 bool *aResult)
 {
   // Jump through hoops to extract the docShell from the range.
   nsCOMPtr<nsIDOMNode> node;
   aRange->GetStartContainer(getter_AddRefs(node));
   nsCOMPtr<nsIDOMDocument> document;
   node->GetOwnerDocument(getter_AddRefs(document));
   nsCOMPtr<mozIDOMWindowProxy> window;
   document->GetDefaultView(getter_AddRefs(window));
   nsCOMPtr<nsIWebNavigation> navNav (do_GetInterface(window));
   nsCOMPtr<nsIDocShell> docShell (do_GetInterface(navNav));
 
   // Set up the arguments needed to check if a range is visible.
   nsCOMPtr<nsIPresShell> presShell (docShell->GetPresShell());
   RefPtr<nsPresContext> presContext = presShell->GetPresContext();
   nsCOMPtr<nsIDOMRange> startPointRange = new nsRange(presShell->GetDocument());
-  *aResult = IsRangeVisible(presShell, presContext, aRange,
-                            aMustBeInViewPort, false,
+  *aResult = IsRangeVisible(presShell, presContext, aRange, aFlushLayout,
                             getter_AddRefs(startPointRange),
                             nullptr);
   return NS_OK;
 }
 
 bool
 nsTypeAheadFind::IsRangeVisible(nsIPresShell *aPresShell,
                                 nsPresContext *aPresContext,
-                                nsIDOMRange *aRange, bool aMustBeInViewPort,
-                                bool aGetTopVisibleLeaf,
+                                nsIDOMRange *aRange,
+                                bool aFlushLayout,
                                 nsIDOMRange **aFirstVisibleRange,
                                 bool *aUsesIndependentSelection)
 {
   NS_ASSERTION(aPresShell && aPresContext && aRange && aFirstVisibleRange, 
                "params are invalid");
 
   // We need to know if the range start is visible.
   // Otherwise, return the first visible range start 
   // in aFirstVisibleRange
+  aRange->CloneRange(aFirstVisibleRange);
 
-  aRange->CloneRange(aFirstVisibleRange);
+  if (aFlushLayout) {
+    aPresShell->FlushPendingNotifications(Flush_Layout);
+  }
+
   nsCOMPtr<nsIDOMNode> node;
   aRange->GetStartContainer(getter_AddRefs(node));
 
   nsCOMPtr<nsIContent> content(do_QueryInterface(node));
   if (!content)
     return false;
 
   nsIFrame *frame = content->GetPrimaryFrame();
-  if (!frame)    
+  if (!frame)
     return false;  // No frame! Not visible then.
 
-  if (!frame->StyleVisibility()->IsVisible())
+  // Having a primary frame doesn't mean that the range is visible inside the
+  // viewport. Do a hit-test to determine that quickly and properly.
+  AutoTArray<nsIFrame*,8> frames;
+  nsIFrame *rootFrame = aPresShell->GetRootFrame();
+  RefPtr<nsRange> range = static_cast<nsRange*>(aRange);
+  RefPtr<mozilla::dom::DOMRectList> rects = range->GetClientRects(true, false);
+  for (uint32_t i = 0; i < rects->Length(); ++i) {
+    RefPtr<mozilla::dom::DOMRect> rect = rects->Item(i);
+    nsRect r(nsPresContext::CSSPixelsToAppUnits((float)rect->X()),
+             nsPresContext::CSSPixelsToAppUnits((float)rect->Y()),
+             nsPresContext::CSSPixelsToAppUnits((float)rect->Width()),
+             nsPresContext::CSSPixelsToAppUnits((float)rect->Height()));
+    nsLayoutUtils::GetFramesForArea(rootFrame, r, frames,
+      nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | nsLayoutUtils::IGNORE_ROOT_SCROLL_FRAME);
+  }
+  if (!frames.Length())
     return false;
 
   // Detect if we are _inside_ a text control, or something else with its own
   // selection controller.
   if (aUsesIndependentSelection) {
     *aUsesIndependentSelection = 
       (frame->GetStateBits() & NS_FRAME_INDEPENDENT_SELECTION);
   }
 
-  // ---- We have a frame ----
-  if (!aMustBeInViewPort)   
-    return true; //  Don't need it to be on screen, just in rendering tree
-
-  // Get the next in flow frame that contains the range start
-  int32_t startRangeOffset, startFrameOffset, endFrameOffset;
-  aRange->GetStartOffset(&startRangeOffset);
-  while (true) {
-    frame->GetOffsets(startFrameOffset, endFrameOffset);
-    if (startRangeOffset < endFrameOffset)
-      break;
-
-    nsIFrame *nextContinuationFrame = frame->GetNextContinuation();
-    if (nextContinuationFrame)
-      frame = nextContinuationFrame;
-    else
-      break;
-  }
-
-  // Set up the variables we need, return true if we can't get at them all
-  const uint16_t kMinPixels  = 12;
-  nscoord minDistance = nsPresContext::CSSPixelsToAppUnits(kMinPixels);
-
-  // Get the bounds of the current frame, relative to the current view.
-  // We don't use the more accurate AccGetBounds, because that is
-  // more expensive and the STATE_OFFSCREEN flag that this is used
-  // for only needs to be a rough indicator
-  nsRectVisibility rectVisibility = nsRectVisibility_kAboveViewport;
-
-  if (!aGetTopVisibleLeaf && !frame->GetRect().IsEmpty()) {
-    rectVisibility =
-      aPresShell->GetRectVisibility(frame,
-                                    nsRect(nsPoint(0,0), frame->GetSize()),
-                                    minDistance);
-
-    if (rectVisibility != nsRectVisibility_kAboveViewport) {
-      return true;
-    }
-  }
-
-  // We know that the target range isn't usable because it's not in the
-  // view port. Move range forward to first visible point,
-  // this speeds us up a lot in long documents
-  nsCOMPtr<nsIFrameEnumerator> frameTraversal;
-  nsCOMPtr<nsIFrameTraversal> trav(do_CreateInstance(kFrameTraversalCID));
-  if (trav)
-    trav->NewFrameTraversal(getter_AddRefs(frameTraversal),
-                            aPresContext, frame,
-                            eLeaf,
-                            false, // aVisual
-                            false, // aLockInScrollView
-                            false, // aFollowOOFs
-                            false  // aSkipPopupChecks
-                            );
-
-  if (!frameTraversal)
-    return false;
-
-  while (rectVisibility == nsRectVisibility_kAboveViewport) {
-    frameTraversal->Next();
-    frame = frameTraversal->CurrentItem();
-    if (!frame)
-      return false;
-
-    if (!frame->GetRect().IsEmpty()) {
-      rectVisibility =
-        aPresShell->GetRectVisibility(frame,
-                                      nsRect(nsPoint(0,0), frame->GetSize()),
-                                      minDistance);
-    }
-  }
-
-  if (frame) {
-    nsCOMPtr<nsIDOMNode> firstVisibleNode = do_QueryInterface(frame->GetContent());
-
-    if (firstVisibleNode) {
-      frame->GetOffsets(startFrameOffset, endFrameOffset);
-      (*aFirstVisibleRange)->SetStart(firstVisibleNode, startFrameOffset);
-      (*aFirstVisibleRange)->SetEnd(firstVisibleNode, endFrameOffset);
-    }
-  }
-
-  return false;
+  return true;
 }
 
 already_AddRefed<nsIPresShell>
 nsTypeAheadFind::GetPresShell()
 {
   if (!mPresShell)
     return nullptr;
 
--- a/toolkit/components/typeaheadfind/nsTypeAheadFind.h
+++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.h
@@ -51,18 +51,18 @@ protected:
   void RangeStartsInsideLink(nsIDOMRange *aRange, nsIPresShell *aPresShell, 
                              bool *aIsInsideLink, bool *aIsStartingLink);
 
   void GetSelection(nsIPresShell *aPresShell, nsISelectionController **aSelCon, 
                     nsISelection **aDomSel);
   // *aNewRange may not be collapsed.  If you want to collapse it in a
   // particular way, you need to do it yourself.
   bool IsRangeVisible(nsIPresShell *aPresShell, nsPresContext *aPresContext,
-                        nsIDOMRange *aRange, bool aMustBeVisible, 
-                        bool aGetTopVisibleLeaf, nsIDOMRange **aNewRange,
+                        nsIDOMRange *aRange, bool aFlushLayout,
+                        nsIDOMRange **aNewRange,
                         bool *aUsesIndependentSelection);
   nsresult FindItNow(nsIPresShell *aPresShell, bool aIsLinksOnly,
                      bool aIsFirstVisiblePreferred, bool aFindPrev,
                      uint16_t* aResult);
   nsresult GetSearchContainers(nsISupports *aContainer,
                                nsISelectionController *aSelectionController,
                                bool aIsFirstVisiblePreferred,
                                bool aFindPrev, nsIPresShell **aPresShell,
--- a/toolkit/modules/FinderIterator.jsm
+++ b/toolkit/modules/FinderIterator.jsm
@@ -568,17 +568,18 @@ this.FinderIterator = {
       // Don't count matches in hidden frames.
       let frameEl = frame && frame.frameElement;
       if (!frameEl)
         continue;
       // Construct a range around the frame element to check its visiblity.
       let range = window.document.createRange();
       range.setStart(frameEl, 0);
       range.setEnd(frameEl, 0);
-      if (!finder._fastFind.isRangeVisible(range, this._getDocShell(range), true))
+      // Pass `true` to flush layout.
+      if (!finder._fastFind.isRangeVisible(range, true))
         continue;
       // All conditions pass, so push the current frame and its children on the
       // stack.
       frames.push(frame, ...this._collectFrames(frame, finder));
     }
 
     return frames;
   },