Closed
Bug 1496833
Opened 6 years ago
Closed 6 years ago
Add a reftest that depends on ancestor intrinsics being cleared on bsize changes.
Categories
(Core :: Layout, enhancement)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(1 file)
It was added to handle the flex container frame clearing up its cached state on the flex item. But that should no longer be needed since recent changes by dholbert made us not clear the whole set of cached reflows, but just the particular flex child item whose intrinsic size changed.
Assignee | ||
Comment 1•6 years ago
|
||
Bug 1490890 made the flex container not responsible for clearing the cached reflows, and further fixes to the cached stuff have landed, so this should be possible now.
Updated•6 years ago
|
Attachment #9014918 -
Attachment description: Ancestor intrinsics no longer need to be cleared. → Ancestor intrinsics no longer need to be cleared on bsize changes.
Comment 2•6 years ago
|
||
Hmm... so suppose we have the following DOM: <div style="display:flex; flex-direction:column"> <div id="flexItem" style="height:auto"> <div="myHeightChanges">...</div> </div> </div> ...and suppose #myHeightChanges undergoes a height change. Will we still clear the cached measurement for #flexItem, after your patch here? In current mozilla-central, I imagine we do clear the cached measurement, because the height change (on the innermost div) will trigger nsChangeHint_ClearAncestorIntrinsic, which makes us call nsFrame::MarkIntrinsicISizesDirty up the parent chain, which makes #flexItem delete its cached size. But with the attached patch, it seems like that won't happen anymore, so I don't immediately see how that measurement would get purged... (I might be forgetting a piece of how this works, though.)
Flags: needinfo?(emilio)
Assignee | ||
Comment 3•6 years ago
|
||
We use the computed size as part of the cache key right? I think that should avoid us using the cached reflow in that case.
Flags: needinfo?(emilio)
Comment 4•6 years ago
|
||
In the scenario that I laid out, the ReflowInput::ComputedBSize() would be nscoord_MAX. (The actual measured bsize -- based on the child's size -- is the thing we're trying to measure + cache.)
Comment 5•6 years ago
|
||
(in other words: the BSize that's part of the cache key would be nscoord_MAX, and the bsize that undergoes a change would be the cache *value*, but we only find out that it's changed if we purge the cache, e.g. due to noticing that one of our descendants underwent a bsize change via this nsChangeHint.)
Updated•6 years ago
|
Attachment #9014918 -
Attachment description: Ancestor intrinsics no longer need to be cleared on bsize changes. → Add a reftest that depends on ancestor intrinsics being cleared on bsize changes.
Assignee | ||
Comment 6•6 years ago
|
||
You're right! I repurposed this to add a reftest instead.
Summary: Should not need to clear ancestor intrinsic sizes due to a bsize change anymore. → Add a reftest that depends on ancestor intrinsics being cleared on bsize changes.
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/f34806131069 Add a reftest that depends on ancestor intrinsics being cleared on bsize changes. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13439 for changes under testing/web-platform/tests
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f34806131069
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Upstream PR merged
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/13439 * Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/KxESzcxbQ2m7jgWSLE7LFw)
You need to log in
before you can comment on or make changes to this bug.
Description
•