Closed Bug 1002482 Opened 10 years ago Closed 10 years ago

Footgun: NotifyLayersUpdated calls mFrameMetrics.IsDefault() after modifying mFrameMetrics

Categories

(Core :: Panning and Zooming, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: kats, Assigned: kats)

Details

Attachments

(2 files, 1 obsolete file)

This doesn't cause any bugs at the moment, but only because mTransformScale isn't checked as part of FrameMetrics::operator==. I don't know if that's intentional or not but this feels like a footgun to me and should be fixed.
Attached patch Remove footgunSplinter Review
Attachment #8413762 - Flags: review?(botond)
Attached patch Log mTransformScale (obsolete) — Splinter Review
Attachment #8413764 - Flags: review?(botond)
Attachment #8413762 - Flags: review?(botond) → review+
Comment on attachment 8413764 [details] [diff] [review]
Log mTransformScale

Review of attachment 8413764 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ +65,5 @@
>  // #define APZC_LOG(...) printf_stderr("APZC: " __VA_ARGS__)
>  #define APZC_LOG_FM(fm, prefix, ...) \
>    APZC_LOG(prefix ":" \
>             " i=(%ld %lld) cb=(%d %d %d %d) rcs=(%.3f %.3f) dp=(%.3f %.3f %.3f %.3f) dpm=(%.3f %.3f %.3f %.3f) um=%d " \
> +           "v=(%.3f %.3f %.3f %.3f) s=(%.3f %.3f) sr=(%.3f %.3f %.3f %.3f) z=(%.3f %.3f %.3f %.3f %.3f) u=(%d %lu)\n", \

I always have to check this code to see what the different components of 'z' are. Perhaps it might make sense to label its components individually?
Attachment #8413764 - Flags: review?(botond) → review+
Updated as requested, carrying r+
Attachment #8413764 - Attachment is obsolete: true
Attachment #8413775 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ac846d851531
https://hg.mozilla.org/mozilla-central/rev/7f902da7deba
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.