Commit bf9563fb authored by Thiemo Nagel's avatar Thiemo Nagel Committed by Chromium LUCI CQ

Revert "IO: Use pre-margin target rect for an empty check for intersection threshold"

This reverts commit 258f8486.

Reason for revert: failure in browser_tests/TabStripTabListTest.All

As unlikely as it looks (to me), I've bisected test failure to this CL

Builder Linux Tests (dbg)(1): https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests%20%28dbg%29%281%29

Original change's description:
> IO: Use pre-margin target rect for an empty check for intersection threshold
>
> This patch ensures that we use a pre-margin version of the target rect
> if we are checking whether it was empty. This is to make sure that we
> compute a "1" intersection ratio in cases where an ancestor of a
> margin-padded target causes it to be empty.
>
> In this case, we should report 1 intersection, because we are intersecting
> and target is an empty rect. However, because we padded the target
> with a margin, we ended up doing the wrong check and ultimately compute
> 0 intersection in a different conditional branch (before this patch).
>
> This patch restores correct behavior.
>
> R=​szager@chromium.org, chrishtr@chromium.org
> Fixed: 1156815
>
> Change-Id: I1697388dd127737a0203d1c3ff15095de4ab41df
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580046
> Reviewed-by: Stefan Zager <szager@chromium.org>
> Commit-Queue: vmpstr <vmpstr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#834930}

TBR=szager@chromium.org,vmpstr@chromium.org,chrishtr@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Idf7927cbd535d91db51da4f3f97280c2359e9b97
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582059Reviewed-by: default avatarThiemo Nagel <tnagel@chromium.org>
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835216}
parent bc377ca1
......@@ -97,27 +97,24 @@ PhysicalRect InitializeRootRect(const LayoutObject* root,
return result;
}
// Return the bounding box of target in target's own coordinate system, also
// return a bool indicating whether the target rect before margin application
// was empty.
std::pair<PhysicalRect, bool> InitializeTargetRect(const LayoutObject* target,
unsigned flags,
const Vector<Length>& margin,
const LayoutObject* root) {
std::pair<PhysicalRect, bool> result;
// Return the bounding box of target in target's own coordinate system
PhysicalRect InitializeTargetRect(const LayoutObject* target,
unsigned flags,
const Vector<Length>& margin,
const LayoutObject* root) {
PhysicalRect result;
if ((flags & IntersectionGeometry::kShouldUseReplacedContentRect) &&
target->IsLayoutEmbeddedContent()) {
result.first = To<LayoutEmbeddedContent>(target)->ReplacedContentRect();
result = To<LayoutEmbeddedContent>(target)->ReplacedContentRect();
} else if (target->IsBox()) {
result.first = PhysicalRect(To<LayoutBox>(target)->BorderBoundingBox());
result = PhysicalRect(To<LayoutBox>(target)->BorderBoundingBox());
} else if (target->IsLayoutInline()) {
result.first = target->AbsoluteToLocalRect(
result = target->AbsoluteToLocalRect(
PhysicalRect::EnclosingRect(target->AbsoluteBoundingBoxFloatRect()));
} else {
result.first = To<LayoutText>(target)->PhysicalLinesBoundingBox();
result = To<LayoutText>(target)->PhysicalLinesBoundingBox();
}
result.second = result.first.IsEmpty();
ApplyMargin(result.first, margin, root->StyleRef().EffectiveZoom(),
ApplyMargin(result, margin, root->StyleRef().EffectiveZoom(),
InitializeRootRect(root, {} /* margin */));
return result;
}
......@@ -296,30 +293,22 @@ void IntersectionGeometry::ComputeGeometry(const RootGeometry& root_geometry,
// root_rect_ is in root's coordinate system
// The coordinate system for unclipped_intersection_rect_ depends on whether
// or not we can use previously cached geometry...
bool pre_margin_target_rect_is_empty;
if (ShouldUseCachedRects()) {
target_rect_ = cached_rects->local_target_rect;
pre_margin_target_rect_is_empty =
cached_rects->pre_margin_target_rect_is_empty;
// The cached intersection rect has already been mapped/clipped up to the
// root, except that the root's scroll offset and overflow clip have not
// been applied.
unclipped_intersection_rect_ =
cached_rects->unscrolled_unclipped_intersection_rect;
} else {
std::tie(target_rect_, pre_margin_target_rect_is_empty) =
InitializeTargetRect(target, flags_, target_margin, root);
target_rect_ = InitializeTargetRect(target, flags_, target_margin, root);
// We have to map/clip target_rect_ up to the root, so we begin with the
// intersection rect in target's coordinate system. After ClipToRoot, it
// will be in root's coordinate system.
unclipped_intersection_rect_ = target_rect_;
}
if (cached_rects) {
if (cached_rects)
cached_rects->local_target_rect = target_rect_;
cached_rects->pre_margin_target_rect_is_empty =
pre_margin_target_rect_is_empty;
}
root_rect_ = root_geometry.local_root_rect;
bool does_intersect =
......@@ -409,14 +398,7 @@ void IntersectionGeometry::ComputeGeometry(const RootGeometry& root_geometry,
if (does_intersect) {
const PhysicalRect& comparison_rect =
ShouldTrackFractionOfRoot() ? root_rect_ : target_rect_;
// Note that if we are checking whether target is empty, we have to use
// pre_margin_target_rect_is_empty for the check. This is because if we are
// using target margin, we may have inflated the rect, so direct empty check
// will be wrong.
bool comparison_rect_empty = ShouldTrackFractionOfRoot()
? root_rect_.IsEmpty()
: pre_margin_target_rect_is_empty;
if (comparison_rect_empty) {
if (comparison_rect.IsEmpty()) {
intersection_ratio_ = 1;
} else {
const PhysicalSize& intersection_size = intersection_rect_.size;
......
......@@ -63,8 +63,6 @@ class CORE_EXPORT IntersectionGeometry {
// True iff unscrolled_unclipped_intersection_rect actually intersects the
// root, as defined by edge-inclusive intersection rules.
bool does_intersect;
// True iff the target rect before any margins were applied was empty
bool pre_margin_target_rect_is_empty;
// Invalidation flag
bool valid;
};
......
<!doctype HTML>
<html>
<meta charset="utf8">
<title>CSS Content Visibility: auto in overflow hidden paints (reference)</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<div>content</div>
<!doctype HTML>
<meta charset="utf8">
<title>CSS Content Visibility: auto in overflow hidden paints</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<link rel="match" href="content-visibility-079-ref.html">
<meta name="assert" content="content-visibility auto element paints in an overflow hidden element that is not sized">
<style>
.auto { content-visibility: auto; }
.overflow { overflow: hidden; }
</style>
<div class=overflow>
<div class=auto>content</div>
</div>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment