[webkit-changes] [WebKit/WebKit] beefa0: Check rendererCanHaveTrimmedMargin before calling ...

Thu Apr 6 12:49:51 PDT 2023

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: beefa0360c1e5611074acc823e60878b9be936db
      https://github.com/WebKit/WebKit/commit/beefa0360c1e5611074acc823e60878b9be936db
  Author: Sammy Gill <sammy.gill at apple.com>
  Date:   2023-04-06 (Thu, 06 Apr 2023)

  Changed paths:
    A LayoutTests/css3/flexbox/flex-item-margin-top-no-crash-expected.html
    A LayoutTests/css3/flexbox/flex-item-margin-top-no-crash.html
    M Source/WebCore/css/ComputedStyleExtractor.cpp
    M Source/WebCore/rendering/RenderBox.cpp

  Log Message:
  -----------
  Check rendererCanHaveTrimmedMargin before calling box->hasTrimmedMargin
https://bugs.webkit.org/show_bug.cgi?id=254859
rdar://problem/107502795

Reviewed by Alan Baradlay.

In this example we will trigger ASSERT(!needsLayout()) inside of
RenderBox::hasTrimmedMargin. This is because computing the margin on
the item is not layout dependent (isLayoutDependent returns false)
and we end up calling into box->hasTrimmedMargin before layout has
occurred.

<style>
.flexbox {
    display: flex;
}
.flex-item {
    width: 50px;
    height: 50px;
    margin-top: 10px;
    background-color: green;
}
</style>
<body>
    <div class="flexbox">
        <div id="flex-item" class="flex-item"></div>
    </div>
<script>
    window.getComputedStyle(document.getElementById("flex-item")).marginTop;
</script>
</body>

Before calling hasTrimmedMargin we should check for the same conditions
that would make isLayoutDependent return true. To accomplish this, I
renamed rendererContainingBlockHasMarginTrim to rendererCanHaveTrimmedMargin
and removed the asserts that were in there. These asserts were
unnecessary because they were the same ones being checked inside of
hasTrimmedMargin. Now we can use this helper function inside of
isLayoutDependent and to guard the call to hasTrimmedMargin. Another
benefit of this is that as we iterate on the computed values for
trimmed margins in different layout contexts we can just change this
one function rather than sprinkling the same checks around each of
the margin types.

* LayoutTests/css3/flexbox/flex-item-margin-top-no-crash-expected.html: Added.
* LayoutTests/css3/flexbox/flex-item-margin-top-no-crash.html: Added.
* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::rendererCanHaveTrimmedMargin):
(WebCore::isLayoutDependent):
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle):
(WebCore::rendererContainingBlockHasMarginTrim): Deleted.
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::hasTrimmedMargin const):

Canonical link: https://commits.webkit.org/262679@main