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

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Remove footgun

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Log mTransformScale

Comment 3

[Botond Ballo [:botond]]

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?

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Log mTransformScale and clarify which values are what

Updated as requested, carrying r+

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac846d851531
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f902da7deba

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ac846d851531
https://hg.mozilla.org/mozilla-central/rev/7f902da7deba