Bug 1622894 - Refactor a bit nsDOMWindowUtils::ZoomToFocusedInput. r=botond
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 18 Mar 2020 20:29:19 +0000
changeset 519553 c8150539058d1d77e3e5f507f88593c0fb5312d2
parent 519552 97879e1e1dcb4c0577bb4ed4b7f803a9c236fc22
child 519554 b50bdc37f8f2b688b218e4075c9dc3a60c72e096
push id110566
push userealvarez@mozilla.com
push dateThu, 19 Mar 2020 09:19:22 +0000
treeherderautoland@041c310f2bf7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1622894, 656036
milestone76.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 1622894 - Refactor a bit nsDOMWindowUtils::ZoomToFocusedInput. r=botond This refactoring: * Removes the "avoid focusing a fixed subtree" condition, because it doesn't make sense after bug 656036. * Avoids keeping nsIFrame pointers across calls to ScrollContentIntoView(), which can flush layout and thus is unsafe. Differential Revision: https://phabricator.services.mozilla.com/D67220
dom/base/nsDOMWindowUtils.cpp
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@@ -2474,102 +2474,88 @@ nsDOMWindowUtils::DisableApzForElement(E
 }
 
 NS_IMETHODIMP
 nsDOMWindowUtils::ZoomToFocusedInput() {
   nsIWidget* widget = GetWidget();
   if (!widget) {
     return NS_OK;
   }
+
   // If APZ is not enabled, this function is a no-op.
+  //
+  // FIXME(emilio): This is not quite true anymore now that we also
+  // ScrollIntoView() too...
   if (!widget->AsyncPanZoomEnabled()) {
     return NS_OK;
   }
 
   nsFocusManager* fm = nsFocusManager::GetFocusManager();
   if (!fm) {
     return NS_OK;
   }
 
-  nsCOMPtr<nsIContent> content = fm->GetFocusedElement();
-  if (!content) {
+  RefPtr<Element> element = fm->GetFocusedElement();
+  if (!element) {
     return NS_OK;
   }
 
   RefPtr<PresShell> presShell =
-      APZCCallbackHelper::GetRootContentDocumentPresShellForContent(content);
+      APZCCallbackHelper::GetRootContentDocumentPresShellForContent(element);
   if (!presShell) {
     return NS_OK;
   }
 
-  nsIScrollableFrame* rootScrollFrame =
-      presShell->GetRootScrollFrameAsScrollable();
-  if (!rootScrollFrame) {
-    return NS_OK;
-  }
-
-  nsIFrame* currentFrame = content->GetPrimaryFrame();
-  nsIFrame* rootFrame = presShell->GetRootFrame();
-  nsIFrame* scrolledFrame = rootScrollFrame->GetScrolledFrame();
-  bool isFixedPos = true;
-
-  while (currentFrame) {
-    if (currentFrame == rootFrame) {
-      break;
-    }
-    if (currentFrame == scrolledFrame) {
-      // we are in the rootScrollFrame so this element is not fixed
-      isFixedPos = false;
-      break;
-    }
-    currentFrame = nsLayoutUtils::GetCrossDocParentFrame(currentFrame);
-  }
-
-  if (isFixedPos) {
-    // We didn't find the scrolledFrame in our parent frames so this content
-    // must be fixed position. Zooming into fixed position content doesn't make
-    // sense so just return with out panning and zooming.
+  if (!element->GetPrimaryFrame()) {
     return NS_OK;
   }
 
   Document* document = presShell->GetDocument();
   if (!document) {
     return NS_OK;
   }
 
   uint32_t presShellId;
   ScrollableLayerGuid::ViewID viewId;
-  if (APZCCallbackHelper::GetOrCreateScrollIdentifiers(
+  if (!APZCCallbackHelper::GetOrCreateScrollIdentifiers(
           document->GetDocumentElement(), &presShellId, &viewId)) {
-    uint32_t flags = layers::DISABLE_ZOOM_OUT;
-    if (!Preferences::GetBool("formhelper.autozoom")) {
-      flags |= layers::PAN_INTO_VIEW_ONLY;
-    } else {
-      flags |= layers::ONLY_ZOOM_TO_DEFAULT_SCALE;
-    }
-
-    // The content may be inside a scrollable subframe inside a non-scrollable
-    // root content document. In this scenario, we want to ensure that the
-    // main-thread side knows to scroll the content into view before we get
-    // the bounding content rect and ask APZ to adjust the visual viewport.
-    presShell->ScrollContentIntoView(
-        content, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
-        ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
-        ScrollFlags::ScrollOverflowHidden);
-
-    CSSRect bounds =
-        nsLayoutUtils::GetBoundingContentRect(content, rootScrollFrame);
-    if (bounds.IsEmpty()) {
-      // Do not zoom on empty bounds. Bail out.
-      return NS_OK;
-    }
-    bounds.Inflate(15.0f, 0.0f);
-    widget->ZoomToRect(presShellId, viewId, bounds, flags);
-  }
-
+    return NS_OK;
+  }
+
+  uint32_t flags = layers::DISABLE_ZOOM_OUT;
+  if (!Preferences::GetBool("formhelper.autozoom")) {
+    flags |= layers::PAN_INTO_VIEW_ONLY;
+  } else {
+    flags |= layers::ONLY_ZOOM_TO_DEFAULT_SCALE;
+  }
+
+  // The content may be inside a scrollable subframe inside a non-scrollable
+  // root content document. In this scenario, we want to ensure that the
+  // main-thread side knows to scroll the content into view before we get
+  // the bounding content rect and ask APZ to adjust the visual viewport.
+  presShell->ScrollContentIntoView(
+      element, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
+      ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
+      ScrollFlags::ScrollOverflowHidden);
+
+  nsIScrollableFrame* rootScrollFrame =
+      presShell->GetRootScrollFrameAsScrollable();
+  if (!rootScrollFrame) {
+    return NS_OK;
+  }
+
+  CSSRect bounds =
+      nsLayoutUtils::GetBoundingContentRect(element, rootScrollFrame);
+  if (bounds.IsEmpty()) {
+    // Do not zoom on empty bounds. Bail out.
+    return NS_OK;
+  }
+
+  bounds.Inflate(15.0f, 0.0f);
+  widget->ZoomToRect(presShellId, viewId, bounds, flags);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMWindowUtils::ComputeAnimationDistance(Element* aElement,
                                            const nsAString& aProperty,
                                            const nsAString& aValue1,
                                            const nsAString& aValue2,