nsStyleSides comparison always assume Calc values are different

RESOLVED FIXED in mozilla17

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

unspecified
mozilla17
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 647374 [details] [diff] [review]
Compare calc values

This is causing unnecessary invalidation in Gaia.
Attachment #647374 - Flags: review?(roc)
Attachment #647374 - Flags: review?(roc) → review?(dbaron)
Comment on attachment 647374 [details] [diff] [review]
Compare calc values

Could you just get rid of this macro and construct an nsStyleCoord for each side?  nsStyleCoord::nsStyleCoord(const nsStyleUnion&, nsStyleUnit) is inline and quite cheap, and then this logic will be in one place (nsStyleCoord::operator==) rather than two.
Comment on attachment 647374 [details] [diff] [review]
Compare calc values

Good catch, though, but I'd like this not to happen in the future (see previous comment).

(Alternatively, if you think that there would be a performance difference, you could refactor the macro so it's used by both places and there's only one thing to change.  But I don't think it's worth it.)
Attachment #647374 - Flags: review?(dbaron) → review-
(Assignee)

Comment 3

5 years ago
Created attachment 647393 [details] [diff] [review]
Compare calc values v2

Seems reasonable, updated the patch
Attachment #647374 - Attachment is obsolete: true
Attachment #647393 - Flags: review?(dbaron)
Comment on attachment 647393 [details] [diff] [review]
Compare calc values v2

r=dbaron.  Thanks for finding and fixing this.
Attachment #647393 - Flags: review?(dbaron) → review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a57c83db51a7
(Assignee)

Updated

5 years ago
Assignee: nobody → matt.woodrow
https://hg.mozilla.org/mozilla-central/rev/a57c83db51a7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(Assignee)

Comment 7

5 years ago
Created attachment 647659 [details] [diff] [review]
Mochitest for this bug
Attachment #647659 - Flags: review?(dbaron)
Comment on attachment 647659 [details] [diff] [review]
Mochitest for this bug

r=dbaron
Attachment #647659 - Flags: review?(dbaron) → review+
You need to log in before you can comment on or make changes to this bug.