ZoomForDSF: Remove pixel snapped behavior for offset/client dimensions.

Instead of pixel snapping, the code now rounds the post-zoom values. Fixed: 1308406 (cherry picked from commit ea147e5) Change-Id: I43f56cd4dba1a2f782d1539251155e64fa6dbc6c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3541546 Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Vladimir Levin <vmpstr@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#986003} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561498 Auto-Submit: Vladimir Levin <vmpstr@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/branch-heads/4951@{#270} Cr-Branched-From: 27de622-refs/heads/main@{#982481}
chromium · Mar 30, 2022 · 220bef2 · 220bef2
1 parent 0a73a2e
commit 220bef2
Show file tree

Hide file tree

Showing 15 changed files with 116 additions and 74 deletions.
diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
@@ -1221,9 +1221,10 @@ int Element::OffsetLeft() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    return AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetLeft(OffsetParent()),
-        layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(
+               layout_object->OffsetLeft(OffsetParent()),
+               layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
@@ -1232,9 +1233,10 @@ int Element::OffsetTop() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    return AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetTop(OffsetParent()),
-        layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(
+               layout_object->OffsetTop(OffsetParent()),
+               layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
@@ -1243,9 +1245,9 @@ int Element::OffsetWidth() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    return AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetWidth(OffsetParent()),
-        layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(layout_object->OffsetWidth(),
+                                                   layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
@@ -1254,9 +1256,9 @@ int Element::OffsetHeight() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    return AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetHeight(OffsetParent()),
-        layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(
+               layout_object->OffsetHeight(), layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
@@ -1274,8 +1276,9 @@ int Element::clientLeft() {
                                             DocumentUpdateReason::kJavaScript);
 
   if (const auto* layout_object = GetLayoutBox()) {
-    return AdjustForAbsoluteZoom::AdjustInt(layout_object->ClientLeft().Round(),
-                                            layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(layout_object->ClientLeft(),
+                                                   layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
@@ -1285,8 +1288,9 @@ int Element::clientTop() {
                                             DocumentUpdateReason::kJavaScript);
 
   if (const auto* layout_object = GetLayoutBox()) {
-    return AdjustForAbsoluteZoom::AdjustInt(layout_object->ClientTop().Round(),
-                                            layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(layout_object->ClientTop(),
+                                                   layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
@@ -1392,9 +1396,11 @@ int Element::clientWidth() {
         // OverflowClipRect() may return infinite along a particular axis if
         // |layout_view| is not a scroll-container.
         DCHECK(layout_view->IsScrollContainer());
-        int result = AdjustForAbsoluteZoom::AdjustInt(
-            layout_view->OverflowClipRect(PhysicalOffset()).Width().Round(),
-            layout_view->StyleRef());
+        int result =
+            AdjustForAbsoluteZoom::AdjustLayoutUnit(
+                layout_view->OverflowClipRect(PhysicalOffset()).Width(),
+                layout_view->StyleRef())
+                .Round();
         RecordScrollbarSizeForStudy(result, /* is_width= */ true,
                                     /* is_offset= */ false);
         return result;
@@ -1412,9 +1418,10 @@ int Element::clientWidth() {
 
   int result = 0;
   if (const auto* layout_object = GetLayoutBox()) {
-    result = AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedClientWidthWithTableSpecialBehavior(),
-        layout_object->StyleRef());
+    result = AdjustForAbsoluteZoom::AdjustLayoutUnit(
+                 layout_object->ClientWidthWithTableSpecialBehavior(),
+                 layout_object->StyleRef())
+                 .Round();
     RecordScrollbarSizeForStudy(result, /* is_width= */ true,
                                 /* is_offset= */ false);
   }
@@ -1439,9 +1446,11 @@ int Element::clientHeight() {
         // OverflowClipRect() may return infinite along a particular axis if
         // |layout_view| is not a scroll-container.
         DCHECK(layout_view->IsScrollContainer());
-        int result = AdjustForAbsoluteZoom::AdjustInt(
-            layout_view->OverflowClipRect(PhysicalOffset()).Height().Round(),
-            layout_view->StyleRef());
+        int result =
+            AdjustForAbsoluteZoom::AdjustLayoutUnit(
+                layout_view->OverflowClipRect(PhysicalOffset()).Height(),
+                layout_view->StyleRef())
+                .Round();
         RecordScrollbarSizeForStudy(result, /* is_width= */ false,
                                     /* is_offset= */ false);
         return result;
@@ -1459,9 +1468,10 @@ int Element::clientHeight() {
 
   int result = 0;
   if (const auto* layout_object = GetLayoutBox()) {
-    result = AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedClientHeightWithTableSpecialBehavior(),
-        layout_object->StyleRef());
+    result = AdjustForAbsoluteZoom::AdjustLayoutUnit(
+                 layout_object->ClientHeightWithTableSpecialBehavior(),
+                 layout_object->StyleRef())
+                 .Round();
     RecordScrollbarSizeForStudy(result, /* is_width= */ false,
                                 /* is_offset= */ false);
   }
@@ -1669,8 +1679,8 @@ int Element::scrollWidth() {
   }
 
   if (LayoutBox* box = GetLayoutBox()) {
-    return AdjustForAbsoluteZoom::AdjustInt(box->PixelSnappedScrollWidth(),
-                                            box);
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(box->ScrollWidth(), *box)
+        .Round();
   }
   return 0;
 }
@@ -1692,8 +1702,8 @@ int Element::scrollHeight() {
   }
 
   if (LayoutBox* box = GetLayoutBox()) {
-    return AdjustForAbsoluteZoom::AdjustInt(box->PixelSnappedScrollHeight(),
-                                            box);
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(box->ScrollHeight(), *box)
+        .Round();
   }
   return 0;
 }

diff --git a/third_party/blink/renderer/core/html/html_element.cc b/third_party/blink/renderer/core/html/html_element.cc
@@ -1786,36 +1786,35 @@ void HTMLElement::HandleKeypressEvent(KeyboardEvent& event) {
 int HTMLElement::offsetLeftForBinding() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
-  Element* offset_parent = unclosedOffsetParent();
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    return AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetLeft(offset_parent),
-        layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(
+               layout_object->OffsetLeft(unclosedOffsetParent()),
+               layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
 
 int HTMLElement::offsetTopForBinding() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
-  Element* offset_parent = unclosedOffsetParent();
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    return AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetTop(offset_parent),
-        layout_object->StyleRef());
+    return AdjustForAbsoluteZoom::AdjustLayoutUnit(
+               layout_object->OffsetTop(unclosedOffsetParent()),
+               layout_object->StyleRef())
+        .Round();
   }
   return 0;
 }
 
 int HTMLElement::offsetWidthForBinding() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
-  Element* offset_parent = unclosedOffsetParent();
   int result = 0;
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    result = AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetWidth(offset_parent),
-        layout_object->StyleRef());
+    result = AdjustForAbsoluteZoom::AdjustLayoutUnit(
+                 layout_object->OffsetWidth(), layout_object->StyleRef())
+                 .Round();
     RecordScrollbarSizeForStudy(result, /* isWidth= */ true,
                                 /* is_offset= */ true);
   }
@@ -1826,12 +1825,11 @@ DISABLE_CFI_PERF
 int HTMLElement::offsetHeightForBinding() {
   GetDocument().EnsurePaintLocationDataValidForNode(
       this, DocumentUpdateReason::kJavaScript);
-  Element* offset_parent = unclosedOffsetParent();
   int result = 0;
   if (const auto* layout_object = GetLayoutBoxModelObject()) {
-    result = AdjustForAbsoluteZoom::AdjustInt(
-        layout_object->PixelSnappedOffsetHeight(offset_parent),
-        layout_object->StyleRef());
+    result = AdjustForAbsoluteZoom::AdjustLayoutUnit(
+                 layout_object->OffsetHeight(), layout_object->StyleRef())
+                 .Round();
     RecordScrollbarSizeForStudy(result, /* is_width= */ false,
                                 /* is_offset= */ true);
   }

diff --git a/third_party/blink/renderer/core/layout/layout_box.cc b/third_party/blink/renderer/core/layout/layout_box.cc
@@ -1024,7 +1024,7 @@ int LayoutBox::PixelSnappedClientHeight() const {
   return SnapSizeToPixel(ClientHeight(), Location().Y() + ClientTop());
 }
 
-int LayoutBox::PixelSnappedClientWidthWithTableSpecialBehavior() const {
+LayoutUnit LayoutBox::ClientWidthWithTableSpecialBehavior() const {
   NOT_DESTROYED();
   // clientWidth/Height is the visual portion of the box content, not including
   // borders or scroll bars, but includes padding. And per
@@ -1036,14 +1036,12 @@ int LayoutBox::PixelSnappedClientWidthWithTableSpecialBehavior() const {
   // Currently, Blink doesn't have table wrapper box, and we are supposed to
   // retrieve clientWidth/Height from table wrapper box, not table grid box. So
   // when we retrieve clientWidth/Height, it includes table's border size.
-  LayoutUnit client_width = ClientWidth();
   if (IsTable())
-    client_width += BorderLeft() + BorderRight();
-  return SnapSizeToPixel(client_width, Location().X() + ClientLeft());
+    return ClientWidth() + BorderLeft() + BorderRight();
+  return ClientWidth();
 }
 
-DISABLE_CFI_PERF
-int LayoutBox::PixelSnappedClientHeightWithTableSpecialBehavior() const {
+LayoutUnit LayoutBox::ClientHeightWithTableSpecialBehavior() const {
   NOT_DESTROYED();
   // clientWidth/Height is the visual portion of the box content, not including
   // borders or scroll bars, but includes padding. And per
@@ -1055,10 +1053,9 @@ int LayoutBox::PixelSnappedClientHeightWithTableSpecialBehavior() const {
   // Currently, Blink doesn't have table wrapper box, and we are supposed to
   // retrieve clientWidth/Height from table wrapper box, not table grid box. So
   // when we retrieve clientWidth/Height, it includes table's border size.
-  LayoutUnit client_height = ClientHeight();
   if (IsTable())
-    client_height += BorderTop() + BorderBottom();
-  return SnapSizeToPixel(client_height, Location().Y() + ClientTop());
+    return ClientHeight() + BorderTop() + BorderBottom();
+  return ClientHeight();
 }
 
 int LayoutBox::PixelSnappedOffsetWidth(const Element*) const {

diff --git a/third_party/blink/renderer/core/layout/layout_box.h b/third_party/blink/renderer/core/layout/layout_box.h
@@ -877,8 +877,9 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
   // TODO(crbug.com/962299): This is incorrect in some cases.
   int PixelSnappedClientWidth() const;
   int PixelSnappedClientHeight() const;
-  int PixelSnappedClientWidthWithTableSpecialBehavior() const;
-  int PixelSnappedClientHeightWithTableSpecialBehavior() const;
+
+  LayoutUnit ClientWidthWithTableSpecialBehavior() const;
+  LayoutUnit ClientHeightWithTableSpecialBehavior() const;
 
   // scrollWidth/scrollHeight will be the same as clientWidth/clientHeight
   // unless the object has overflow:hidden/scroll/auto specified and also has

diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
@@ -7755,3 +7755,9 @@ crbug.com/1298633 http/tests/inspector-protocol/network/cross-origin-isolation/c
 # Sheriff 2022-03-16
 crbug.com/1306848 http/tests/inspector-protocol/conversion/attribution-invalid-data.js [ Failure Pass Timeout ]
 crbug.com/1306770 http/tests/inspector-protocol/network/interception-download.js [ Failure Pass ]
+
+# Tests that need updates due to rounding math in layout
+crbug.com/1309975 http/tests/devtools/console/console-viewport-indices.js [ Failure ]
+crbug.com/1309975 http/tests/devtools/console/viewport-testing/console-stick-to-bottom-expand-object.js [ Failure ]
+crbug.com/1309972 external/wpt/css/css-tables/tentative/table-width-redistribution-fixed-padding.html [ Failure ]
+crbug.com/1310332 editing/selection/mouse/drag-user-select-all-contenteditable.html [ Failure ]
diff --git a/..._party/blink/web_tests/css3/flexbox/flex-flow-auto-margins-no-available-space-assert.html b/..._party/blink/web_tests/css3/flexbox/flex-flow-auto-margins-no-available-space-assert.html
@@ -10,7 +10,7 @@
 }
 </style>
 <script src="../../resources/check-layout.js"></script>
-<abbr data-expected-height=30210271>
+<abbr data-expected-height=30210270>
     <input></input>
 </abbr>
 <p> crbug.com/380201: Don't shrink below border/padding when stretching children within a flexbox with no available space.</p>

diff --git a/.../eye-dropper/fast/forms/color-scheme/color/color-picker-appearance-expected.png b/.../eye-dropper/fast/forms/color-scheme/color/color-picker-appearance-expected.png
diff --git a/...ts/platform/linux/fast/forms/color/color-picker-appearance-zoom200-expected.png b/...ts/platform/linux/fast/forms/color/color-picker-appearance-zoom200-expected.png
diff --git a/...k/web_tests/platform/linux/virtual/text-antialias/regional-indicator-symobls-expected.txt b/...k/web_tests/platform/linux/virtual/text-antialias/regional-indicator-symobls-expected.txt
diff --git a/...m/mac-mac11-arm64/fast/forms/color/color-picker-appearance-zoom200-expected.png b/...m/mac-mac11-arm64/fast/forms/color/color-picker-appearance-zoom200-expected.png
diff --git a/...ests/platform/mac/fast/forms/color/color-picker-appearance-zoom200-expected.png b/...ests/platform/mac/fast/forms/color/color-picker-appearance-zoom200-expected.png
diff --git a/...ests/platform/win/fast/forms/color/color-picker-appearance-zoom200-expected.png b/...ests/platform/win/fast/forms/color/color-picker-appearance-zoom200-expected.png
diff --git a/...ests/devtools/console/viewport-testing/console-stick-to-bottom-expand-object-expected.txt b/...ests/devtools/console/viewport-testing/console-stick-to-bottom-expand-object-expected.txt
@@ -0,0 +1,22 @@
+Verifies viewport stick-to-bottom behavior when prompt has space below editable area.
+
+Message count: 150
+
+Running: testExpandLastVisibleObjectRemainsInView
+
+Force selecting index 149
+Is at bottom: true, should stick: true, selected element is fully visible? true
+Expanding object
+Is at bottom: false, should stick: false, selected element is fully visible? false
+Collapsing object
+Is at bottom: true, should stick: false, selected element is fully visible? true
+
+Running: testExpandFirstVisibleObjectRemainsInView
+
+Force selecting index 146
+Is at bottom: false, should stick: false, selected element is fully visible? true
+Expanding object
+Is at bottom: false, should stick: false, selected element is fully visible? true
+Collapsing object
+Is at bottom: false, should stick: false, selected element is fully visible? true
+
diff --git a/...ink/web_tests/platform/win/virtual/text-antialias/regional-indicator-symobls-expected.txt b/...ink/web_tests/platform/win/virtual/text-antialias/regional-indicator-symobls-expected.txt
diff --git a/...ests/devtools/console/viewport-testing/console-stick-to-bottom-expand-object-expected.txt b/...ests/devtools/console/viewport-testing/console-stick-to-bottom-expand-object-expected.txt
@@ -0,0 +1,22 @@
+Verifies viewport stick-to-bottom behavior when prompt has space below editable area.
+
+Message count: 150
+
+Running: testExpandLastVisibleObjectRemainsInView
+
+Force selecting index 149
+Is at bottom: true, should stick: true, selected element is fully visible? true
+Expanding object
+Is at bottom: false, should stick: false, selected element is fully visible? true
+Collapsing object
+Is at bottom: true, should stick: false, selected element is fully visible? true
+
+Running: testExpandFirstVisibleObjectRemainsInView
+
+Force selecting index 146
+Is at bottom: false, should stick: false, selected element is fully visible? true
+Expanding object
+Is at bottom: false, should stick: false, selected element is fully visible? true
+Collapsing object
+Is at bottom: false, should stick: false, selected element is fully visible? true
+