Skip to content

Commit

Permalink
Revert "LayoutNG: Do not call UpdateLogicalWidth/Height for LayoutRep…
Browse files Browse the repository at this point in the history
…laced" in M110 branch

This reverts commit ca2862a.

Reason: See crbug.com/1403580

Original CL description:
> LayoutNG: Do not call UpdateLogicalWidth/Height for LayoutReplaced
>
> We should not call UpdateLogicalWidth() and UpdateLogicalHeight() from
> UpdateLayout() of LayoutReplaced subclasses.
>
> We need ReplacedContentRect() with the last layout result and
> ReplacedContentRect() with the new layout result in UpdateLayout().
> So, this CL updates ReplacedContentRect() and functions used by it
> so that they take source data (border-box size and border-padding
> strut) as arguments, and UpdateLayout() passes
>  - Data from LayoutBox fields, or
>  - Data from BoxLayoutExtraInput.
>
> This CL should have no behavior changes.
>
> Bug: 1353190
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4059933

Bug: 1353190, 1403580
Change-Id: Ibc8c9aedb7b2b1a21346185fee1f6937ec5dfa9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4170010
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Auto-Submit: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#371}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
tkent-google authored and Chromium LUCI CQ committed Jan 17, 2023
1 parent 154fd68 commit be0142a
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 76 deletions.
Expand Up @@ -341,11 +341,9 @@ CursorDirective LayoutEmbeddedContent::GetCursor(const PhysicalOffset& point,
return LayoutReplaced::GetCursor(point, cursor);
}

PhysicalRect LayoutEmbeddedContent::ReplacedContentRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const {
PhysicalRect LayoutEmbeddedContent::ReplacedContentRect() const {
NOT_DESTROYED();
PhysicalRect content_rect = PhysicalContentBoxRectFrom(size, border_padding);
PhysicalRect content_rect = PhysicalContentBoxRect();

// IFrames set as the root scroller should get their size from their parent.
// When scrolling starts so as to hide the URL bar, IFRAME wouldn't resize to
Expand All @@ -364,8 +362,7 @@ PhysicalRect LayoutEmbeddedContent::ReplacedContentRectFrom(
// outside of the child frame. Revisit this when the input system supports
// different |ReplacedContentRect| from |PhysicalContentBoxRect|.
LayoutSize frozen_layout_size = frozen_size->ToLayoutSize();
content_rect =
ComputeReplacedContentRect(size, border_padding, &frozen_layout_size);
content_rect = ComputeReplacedContentRect(&frozen_layout_size);
}

// We don't propagate sub-pixel into sub-frame layout, in other words, the
Expand Down
Expand Up @@ -65,9 +65,7 @@ class CORE_EXPORT LayoutEmbeddedContent : public LayoutReplaced {
PhysicalOffset BorderBoxFromEmbeddedContent(const PhysicalOffset&) const;
gfx::Rect BorderBoxFromEmbeddedContent(const gfx::Rect&) const;

PhysicalRect ReplacedContentRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const final;
PhysicalRect ReplacedContentRect() const final;

void UpdateOnEmbeddedContentViewChange();
void UpdateGeometry(EmbeddedContentView&);
Expand Down
3 changes: 1 addition & 2 deletions third_party/blink/renderer/core/layout/layout_frame.cc
Expand Up @@ -56,8 +56,7 @@ void LayoutFrame::ImageChanged(WrappedImagePtr image, CanDeferInvalidation) {
void LayoutFrame::UpdateLayout() {
NOT_DESTROYED();
// Should respect to BoxLayoutExtraInput.
if (Parent()->IsLayoutNGObject() &&
!RuntimeEnabledFeatures::LayoutNGReplacedNoBoxSettersEnabled()) {
if (Parent()->IsLayoutNGObject()) {
UpdateLogicalWidth();
UpdateLogicalHeight();
}
Expand Down
8 changes: 3 additions & 5 deletions third_party/blink/renderer/core/layout/layout_iframe.cc
Expand Up @@ -46,11 +46,9 @@ void LayoutIFrame::UpdateLayout() {
NOT_DESTROYED();
DCHECK(NeedsLayout());

if (!RuntimeEnabledFeatures::LayoutNGReplacedNoBoxSettersEnabled()) {
UpdateLogicalWidth();
// No kids to layout as a replaced element.
UpdateLogicalHeight();
}
UpdateLogicalWidth();
// No kids to layout as a replaced element.
UpdateLogicalHeight();

ClearLayoutOverflow();
if (!RuntimeEnabledFeatures::LayoutNGUnifyUpdateAfterLayoutEnabled())
Expand Down
47 changes: 10 additions & 37 deletions third_party/blink/renderer/core/layout/layout_replaced.cc
Expand Up @@ -126,10 +126,8 @@ void LayoutReplaced::UpdateLayout() {

PhysicalRect old_content_rect = ReplacedContentRect();

if (!RuntimeEnabledFeatures::LayoutNGReplacedNoBoxSettersEnabled()) {
UpdateLogicalWidth();
UpdateLogicalHeight();
}
UpdateLogicalWidth();
UpdateLogicalHeight();

ClearLayoutOverflow();
ClearSelfNeedsLayoutOverflowRecalc();
Expand All @@ -140,8 +138,7 @@ void LayoutReplaced::UpdateLayout() {

ClearNeedsLayout();

if (ReplacedContentRectFrom(SizeFromNG(), BorderPaddingFromNG()) !=
old_content_rect)
if (ReplacedContentRect() != old_content_rect)
SetShouldDoFullPaintInvalidation();
}

Expand Down Expand Up @@ -717,8 +714,6 @@ absl::optional<PhysicalRect> LayoutReplaced::ComputeObjectViewBoxRect(
}

PhysicalRect LayoutReplaced::ComputeReplacedContentRect(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding,
const LayoutSize* overridden_intrinsic_size) const {
// |intrinsic_size| provides the size of the embedded content rendered in the
// replaced element. This is the reference size that object-view-box applies
Expand Down Expand Up @@ -754,16 +749,14 @@ PhysicalRect LayoutReplaced::ComputeReplacedContentRect(

// If no view box override was applied, then we don't need to adjust the
// view-box paint rect.
if (!view_box) {
return ComputeObjectFitAndPositionRect(size, border_padding,
overridden_intrinsic_size);
}
if (!view_box)
return ComputeObjectFitAndPositionRect(overridden_intrinsic_size);

// Compute the paint rect based on bounds provided by the view box.
DCHECK(!view_box->IsEmpty());
const LayoutSize view_box_size(view_box->Width(), view_box->Height());
const auto view_box_paint_rect =
ComputeObjectFitAndPositionRect(size, border_padding, &view_box_size);
ComputeObjectFitAndPositionRect(&view_box_size);
if (view_box_paint_rect.IsEmpty())
return view_box_paint_rect;

Expand All @@ -787,11 +780,9 @@ PhysicalRect LayoutReplaced::ComputeReplacedContentRect(
}

PhysicalRect LayoutReplaced::ComputeObjectFitAndPositionRect(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding,
const LayoutSize* overridden_intrinsic_size) const {
NOT_DESTROYED();
PhysicalRect content_rect = PhysicalContentBoxRectFrom(size, border_padding);
PhysicalRect content_rect = PhysicalContentBoxRect();
EObjectFit object_fit = StyleRef().GetObjectFit();

if (object_fit == EObjectFit::kFill &&
Expand Down Expand Up @@ -853,20 +844,7 @@ PhysicalRect LayoutReplaced::ComputeObjectFitAndPositionRect(

PhysicalRect LayoutReplaced::ReplacedContentRect() const {
NOT_DESTROYED();
// This function should compute the result with old geometry even if a
// BoxLayoutExtraInput exists.
return ReplacedContentRectFrom(
Size(), NGPhysicalBoxStrut(BorderTop() + PaddingTop(),
BorderRight() + PaddingRight(),
BorderBottom() + PaddingBottom(),
BorderLeft() + PaddingLeft()));
}

PhysicalRect LayoutReplaced::ReplacedContentRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const {
NOT_DESTROYED();
return ComputeReplacedContentRect(size, border_padding);
return ComputeReplacedContentRect();
}

LayoutSize LayoutReplaced::SizeFromNG() const {
Expand All @@ -892,13 +870,8 @@ NGPhysicalBoxStrut LayoutReplaced::BorderPaddingFromNG() const {

PhysicalRect LayoutReplaced::PhysicalContentBoxRectFromNG() const {
NOT_DESTROYED();
return PhysicalContentBoxRectFrom(SizeFromNG(), BorderPaddingFromNG());
}

PhysicalRect LayoutReplaced::PhysicalContentBoxRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const {
NOT_DESTROYED();
LayoutSize size = SizeFromNG();
NGPhysicalBoxStrut border_padding = BorderPaddingFromNG();
return PhysicalRect(
border_padding.left, border_padding.top,
(size.Width() - border_padding.HorizontalSum()).ClampNegativeToZero(),
Expand Down
14 changes: 1 addition & 13 deletions third_party/blink/renderer/core/layout/layout_replaced.h
Expand Up @@ -70,10 +70,7 @@ class CORE_EXPORT LayoutReplaced : public LayoutBox {
// This function returns the local rect of the replaced content. The rectangle
// is in the coordinate space of the element's physical border-box and assumes
// no clipping.
PhysicalRect ReplacedContentRect() const;
virtual PhysicalRect ReplacedContentRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const;
virtual PhysicalRect ReplacedContentRect() const;

// This is used by a few special elements, e.g. <video>, <iframe> to ensure
// a persistent sizing under different subpixel offset, because these
Expand Down Expand Up @@ -177,8 +174,6 @@ class CORE_EXPORT LayoutReplaced : public LayoutBox {
// intrinsic size of the replaced contents, stretch to fit CSS content box
// according to object-fit, object-position and object-view-box.
PhysicalRect ComputeReplacedContentRect(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding,
const LayoutSize* overridden_intrinsic_size = nullptr) const;

LayoutUnit IntrinsicContentLogicalHeight() const override {
Expand Down Expand Up @@ -216,11 +211,6 @@ class CORE_EXPORT LayoutReplaced : public LayoutBox {
// BoxLayoutExtraInput is associated to this box.
NGPhysicalBoxStrut BorderPaddingFromNG() const;

// This returns a local rectangle excluding border_padding.
PhysicalRect PhysicalContentBoxRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const;

private:
// Computes a rect, relative to the element's content's natural size, that
// should be used as the content source when rendering this element. This
Expand All @@ -229,8 +219,6 @@ class CORE_EXPORT LayoutReplaced : public LayoutBox {
const LayoutSize* overridden_intrinsic_size = nullptr) const;

PhysicalRect ComputeObjectFitAndPositionRect(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding,
const LayoutSize* overridden_intrinsic_size) const;

MinMaxSizes PreferredLogicalWidths() const final;
Expand Down
9 changes: 3 additions & 6 deletions third_party/blink/renderer/core/layout/layout_video.cc
Expand Up @@ -201,19 +201,16 @@ void LayoutVideo::UpdatePlayer(bool is_in_layout) {
Layer()->SetNeedsCompositingInputsUpdate();
}

PhysicalRect LayoutVideo::ReplacedContentRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const {
PhysicalRect LayoutVideo::ReplacedContentRect() const {
NOT_DESTROYED();
if (GetDisplayMode() == kVideo) {
// Video codecs may need to restart from an I-frame when the output is
// resized. Round size in advance to avoid 1px snap difference.
return PreSnappedRectForPersistentSizing(
ComputeReplacedContentRect(size, border_padding));
return PreSnappedRectForPersistentSizing(ComputeReplacedContentRect());
}
// If we are displaying the poster image no pre-rounding is needed, but the
// size of the image should be used for fitting instead.
return ComputeReplacedContentRect(size, border_padding, &cached_image_size_);
return ComputeReplacedContentRect(&cached_image_size_);
}

bool LayoutVideo::SupportsAcceleratedRendering() const {
Expand Down
4 changes: 1 addition & 3 deletions third_party/blink/renderer/core/layout/layout_video.h
Expand Up @@ -41,9 +41,7 @@ class CORE_EXPORT LayoutVideo final : public LayoutMedia {

static LayoutSize DefaultSize();

PhysicalRect ReplacedContentRectFrom(
const LayoutSize size,
const NGPhysicalBoxStrut& border_padding) const final;
PhysicalRect ReplacedContentRect() const final;

bool SupportsAcceleratedRendering() const;

Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/layout/ng/ng_block_node.cc
Expand Up @@ -898,7 +898,8 @@ void NGBlockNode::FinishLayout(LayoutBlockFlow* block_flow,
box_->ForceLayout();

#if DCHECK_IS_ON()
if (!RuntimeEnabledFeatures::LayoutNGReplacedNoBoxSettersEnabled()) {
if (!RuntimeEnabledFeatures::LayoutNGReplacedNoBoxSettersEnabled() ||
!box_->IsSVGRoot()) {
// Assert that legacy uses the size NG forces above. But legacy sends
// LayoutUnit to float and back, which can slightly change the result. So
// give a 1px cushion.
Expand Down

0 comments on commit be0142a

Please sign in to comment.