ReflowInput::InitResizeFlags should set them to false up front

RESOLVED FIXED in Firefox 55

Status

()

Core
Layout
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

We can call InitResizeFlags multiple times: from Init, and then from SetComputedWidth/Height.

We should not propagate the old flag values through, because some code in this method assumes the values are false at the start and calls things like IsBResize() to check whether the flag got set by something earlier in the method.

Try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=07e24821802fdb46a1f0e7fbe56f4c029ab517b7 looks pretty green.
Created attachment 8852744 [details] [diff] [review]
Set the resize flags to false at the beginning of InitResizeFlags, so stale values from an old (incorrect) computed bsize won't keep being propagated in cases when we SetComputedWidth or SetComputedHeight on the ReflowInput

MozReview-Commit-ID: TAhHsIAgyb
Attachment #8852744 - Flags: review?(dbaron)
Comment on attachment 8852744 [details] [diff] [review]
Set the resize flags to false at the beginning of InitResizeFlags, so stale values from an old (incorrect) computed bsize won't keep being propagated in cases when we SetComputedWidth or SetComputedHeight on the ReflowInput

r=dbaron on the ReflowInput.cpp changes; I don't think the others belong in this patch.

I don't actually see a codepath where the Is?Resize() calls in this function can depend on something from a previous invocation, though.
Attachment #8852744 - Flags: review?(dbaron) → review+
(still seems like a good idea in order to prevent the scenario in comment 0 from actually happening)
> I don't think the others belong in this patch.

You're right.  They do not.

> I don't actually see a codepath where the Is?Resize() calls in this function
> can depend on something from a previous invocation, though.

You're right that there isn't anything right now.  I guess it was when I had some local changes trying to address our setting bresize too eagerly that I ran into this.

Well, that explains why this should be safe to do.  ;)

Comment 5

8 months ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cf1f3a90672
Set the resize flags to false at the beginning of InitResizeFlags, so stale values from an old (incorrect) computed bsize won't keep being propagated in cases when we SetComputedWidth or SetComputedHeight on the ReflowInput.  r=dbaron

Comment 6

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1cf1f3a90672
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.