<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - bad_optional_access in RenderGrid::updateAutoMarginsInRowAxisIfNeeded"
   href="https://bugs.webkit.org/show_bug.cgi?id=232922#c5">Comment # 5</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - bad_optional_access in RenderGrid::updateAutoMarginsInRowAxisIfNeeded"
   href="https://bugs.webkit.org/show_bug.cgi?id=232922">bug 232922</a>
              from <span class="vcard"><a class="email" href="mailto:gnavamarino@apple.com" title="Gabriel Nava Marino <gnavamarino@apple.com>"> <span class="fn">Gabriel Nava Marino</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=443774&action=diff" name="attach_443774" title="Patch">attachment 443774</a> <a href="attachment.cgi?id=443774&action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=443774&action=review">https://bugs.webkit.org/attachment.cgi?id=443774&action=review</a>

Thank you for the feedback on the stylistic issues.

<span class="quote">>> Source/WebCore/rendering/RenderGrid.cpp:1226
>> +    std::optional<LayoutUnit> availableWidth = child.overridingContainingBlockContentLogicalWidth();

> Although this seems correct I don't get why we have nullopt in the overriding size here. These two methods are called after calling updateGridAreaBreathLogicalSize() which always sets the overrides. That's why we were calling value() directly here without checking the presence of the value. Do you have more info about the exact conditions that trigger the bad access?</span >

I note that these values are set in updateGridAreaBreathLogicalSize(), but later cleared in applyStretchAlignmentToChildIfNeeded:

    // We clear height and width override values because we will decide now whether it's allowed or
    // not, evaluating the conditions which might have changed since the old values were set.
    child.clearOverridingLogicalHeight();
    child.clearOverridingLogicalWidth();

<span class="quote">>> Source/WebCore/rendering/RenderGrid.cpp:1260
>> +    std::optional<LayoutUnit> availableHeight = child.overridingContainingBlockContentLogicalHeight();

> Maybe we could just use value_or(LayoutUnit()) as we already do in other functions of this class to handle the available space. In case of nullopt, we will use the early return already defined for non-positive values.</span >

Thank you, I will use the recommended approach.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>