Bug 224404 - RenderFlexibleBox::m_hasDefiniteHeight should not need to be mutable
Summary: RenderFlexibleBox::m_hasDefiniteHeight should not need to be mutable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-10 04:46 PDT by zalan
Modified: 2021-04-12 08:47 PDT (History)
12 users (show)

See Also:


Attachments
Patch (7.30 KB, patch)
2021-04-10 04:48 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2021-04-10 04:46:40 PDT
fix constness instead.
Comment 1 zalan 2021-04-10 04:48:50 PDT
Created attachment 425684 [details]
Patch
Comment 2 EWS 2021-04-10 05:31:32 PDT
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].
Comment 3 Radar WebKit Bug Importer 2021-04-10 05:32:13 PDT
<rdar://problem/76491463>
Comment 4 Sergio Villar Senin 2021-04-12 03:42:30 PDT
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.
Comment 5 zalan 2021-04-12 08:47:39 PDT
(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).