Last Comment Bug 674494 - Uninitialised variable in ThebesLayerBuffer causes incorrect marking of mDidSelfCopy flag
: Uninitialised variable in ThebesLayerBuffer causes incorrect marking of mDidS...
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Chris Lord [:cwiiis]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-27 03:57 PDT by Chris Lord [:cwiiis]
Modified: 2011-08-18 03:56 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix uninitialised PaintState return variable (3.25 KB, patch)
2011-07-27 04:00 PDT, Chris Lord [:cwiiis]
no flags Details | Diff | Review
Fix uninitialised PaintState return variable (870 bytes, patch)
2011-07-27 04:20 PDT, Chris Lord [:cwiiis]
no flags Details | Diff | Review
Fix uninitialised PaintState return variable by adding a constructor (808 bytes, patch)
2011-08-11 00:32 PDT, Chris Lord [:cwiiis]
matt.woodrow: review+
Details | Diff | Review
Fix uninitialised PaintState return variable by adding a constructor (final?) (726 bytes, patch)
2011-08-15 04:01 PDT, Chris Lord [:cwiiis]
matt.woodrow: review+
Details | Diff | Review

Description Chris Lord [:cwiiis] 2011-07-27 03:57:32 PDT
At the beginning of ThebesLayerBuffer::BeginPaint, the returned 'PaintState' variable is uninitialised. As not all of its fields are set, this can cause the 'mDidSelfCopy' flag to be incorrectly marked.

Patch incoming...
Comment 1 Chris Lord [:cwiiis] 2011-07-27 04:00:22 PDT
Created attachment 548739 [details] [diff] [review]
Fix uninitialised PaintState return variable
Comment 2 Chris Lord [:cwiiis] 2011-07-27 04:20:07 PDT
Created attachment 548742 [details] [diff] [review]
Fix uninitialised PaintState return variable

Sorry, another patch was accidentally combined.
Comment 3 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2011-07-27 13:28:36 PDT
I think I'd rather add a constructor to set mDidSelfCopy to false, good catch though
Comment 4 Chris Lord [:cwiiis] 2011-08-11 00:32:06 PDT
Created attachment 552323 [details] [diff] [review]
Fix uninitialised PaintState return variable by adding a constructor

Updated patch that uses a constructor.
Comment 5 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2011-08-11 13:32:56 PDT
Comment on attachment 552323 [details] [diff] [review]
Fix uninitialised PaintState return variable by adding a constructor

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

::: gfx/layers/ThebesLayerBuffer.h
@@ +114,5 @@
>    struct PaintState {
> +    PaintState()
> +      : mContext(nsnull)
> +      , mRegionToDraw()
> +      , mRegionToInvalidate()

You shouldn't need to explicitly construct these 3, they should all default to the correct values.
Comment 6 Chris Lord [:cwiiis] 2011-08-15 04:01:04 PDT
Created attachment 553142 [details] [diff] [review]
Fix uninitialised PaintState return variable by adding a constructor (final?)

Assuming this is good to go, could you push? I don't have L3 access yet.
Comment 7 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2011-08-15 15:04:42 PDT
Comment on attachment 553142 [details] [diff] [review]
Fix uninitialised PaintState return variable by adding a constructor (final?)

Looks good!

I'll land this on incoming this afternoon.
Comment 8 Matt Woodrow (:mattwoodrow) (PTO until 27 June) 2011-08-17 14:54:56 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/f345becaf55b
Comment 9 Marco Bonardo [::mak] 2011-08-18 03:56:38 PDT
http://hg.mozilla.org/mozilla-central/rev/f345becaf55b

Note You need to log in before you can comment on or make changes to this bug.