Closed Bug 940488 Opened 9 years ago Closed 9 years ago

RotatedBuffer::mDidSelfCopy is uninitialized

Categories

(Core :: Graphics: Layers, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28
blocking-b2g koi+
Tracking Status
firefox26 --- wontfix
firefox27 --- wontfix
firefox28 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

(Keywords: perf, Whiteboard: [c= p=1 s= u=])

Attachments

(1 file, 1 obsolete file)

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?
Attachment #8334641 - Flags: review?(bgirard)
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.
Keywords: checkin-needed
Whiteboard: [c= p= s= u=] → [c= p=1 s= u=]
Update patch title to reflect reviewer.  Carry r+ forward.
Attachment #8334641 - Attachment is obsolete: true
Attachment #8334725 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/59994113d911
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
blocking-b2g: koi? → koi+
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.