| Summary: | RenderFlexibleBox::m_hasDefiniteHeight should not need to be mutable | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | zalan <zalan> | ||||
| Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, koivisto, kondapallykalyan, pdr, simon.fraser, svillar, webkit-bug-importer, zalan | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
zalan
2021-04-10 04:46:40 PDT
Created attachment 425684 [details]
Patch
Committed r275796 (236367@main): <https://commits.webkit.org/236367@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425684 [details]. I know I'm a bit late but I slightly disagree with this change. The reason why we kept it as mutable is because there are several methods like: childMainSizeIsDefinite() childCrossSizeIsDefinite() childHasIntrinsicMainAxisSize() that should be really const as we're just checking some values, they are not meant to change the internal state of the object. The m_hasDefiniteHeight is more a cache than part of the internal state of the object as checking definiteness is sometimes pretty expensive. (In reply to Sergio Villar Senin from comment #4) > I know I'm a bit late but I slightly disagree with this change. The reason > why we kept it as mutable is because there are several methods like: > > childMainSizeIsDefinite() > childCrossSizeIsDefinite() > childHasIntrinsicMainAxisSize() > > that should be really const as we're just checking some values, they are not > meant to change the internal state of the object. The m_hasDefiniteHeight is > more a cache than part of the internal state of the object as checking > definiteness is sometimes pretty expensive. Having mutable to solve issues like this is sometimes an indication of a bigger design problem. I prefer solving them instead (and while this change is not addressing that but it may force you in a long run). |