Closed Bug 940488 Opened 9 years ago Closed 9 years ago
Buffer::m Did Self Copy is uninitialized
When the in-place buffer un-rotation was landed in bug 921212, a new variable called mDidSelfCopy was introduced. It appears, however, that the constructors for RotatedBuffer do not initialize the boolean. This could lead to us perform unnecessary copies in ContentClientDoubleBuffered::SyncFrontBufferToBackBuffer. It seems possible that this might also cause us to present the wrong content since we end up doing this: mBufferRect.MoveTo(mFrontBufferRect.TopLeft()); Instead of: mBufferRect = mFrontBufferRect; If we are silently eating frames, could this explain the FPS regression reports?
Comment on attachment 8334641 [details] [diff] [review] Initialize RotatedBuffer::mDidSelfCopy Review of attachment 8334641 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks good. IMO we should uplift this to 1.2. Any idea why the compiler isn't warning here?
Attachment #8334641 - Flags: review?(bgirard) → review+
(In reply to Benoit Girard (:BenWa) from comment #2) > Any idea why the compiler isn't warning here? From the IRC discussion, this is covered by -Weffc++ flag. Apparently we use -Wall which does not include -Weffc++. Searching online indicates there may be issues with false positives from this flag.
Whiteboard: [c= p= s= u=] → [c= p=1 s= u=]
Update patch title to reflect reviewer. Carry r+ forward.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/df709dd6cf3f Assuming this is B2G-only and therefore doesn't need uplift to Aurora/Beta.
Other platforms could be running into this. It's an uninitialized variable so this is a simple patch that isn't likely to regress. I'd vote for uplift everywhere given the loss risk, high benefit ratio.
You need to log in before you can comment on or make changes to this bug.