UpdateDisplayPortMarginsFromPendingMessages uses the wrong channel

RESOLVED FIXED in Firefox 51

Status

()

Core
Panning and Zooming
P1
normal
RESOLVED FIXED
a year ago
7 months ago

People

(Reporter: billm, Assigned: billm)

Tracking

(Blocks: 1 bug, {regression})

51 Branch
mozilla52
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 unaffected, firefox50 unaffected, firefox51 fixed, firefox52 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

Created attachment 8794415 [details] [diff] [review]
patch

PAPZ is a child protocol of PCompositorBridge, not PContent. Therefore, we need to use the compositor bridge channel instead of the PContent channel. Because of this bug, we're potentially reading garbage into the FrameMetrics and then calling UpdateDisplayPortMarginsForPendingMetrics with it. At worst, maybe we'll crash.
Attachment #8794415 - Flags: review?(bugmail)
Comment on attachment 8794415 [details] [diff] [review]
patch

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

Yikes, good catch, thanks!
Attachment #8794415 - Flags: review?(bugmail) → review+
Please also uplift to 51, as this is a regression in that release.
Blocks: 1289650
status-firefox49: --- → unaffected
status-firefox50: --- → unaffected
status-firefox51: --- → affected
status-firefox52: --- → affected
Keywords: regression
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Version: unspecified → 51 Branch

Comment 3

a year ago
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a376f49314e3
UpdateDisplayPortMarginsFromPendingMessages should use PCompositorBridge channel (r=kats)
Comment on attachment 8794415 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1289650
[User impact if declined]: crashes, poor scrolling performance
[Describe test coverage new/current, TreeHerder]: on m-c
[Risks and why]: low risk, just fixes a regression in code that used to work
[String/UUID change made/needed]: none
Attachment #8794415 - Flags: approval-mozilla-aurora?

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a376f49314e3
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8794415 [details] [diff] [review]
patch

Fix a regression. Take it in 51 aurora.
Attachment #8794415 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 7

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/25c9e1f272a7
status-firefox51: affected → fixed
You need to log in before you can comment on or make changes to this bug.