The default bug view has changed. See this FAQ.

Uninitialised variable in ThebesLayerBuffer causes incorrect marking of mDidSelfCopy flag

RESOLVED FIXED in mozilla9

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cwiiis, Assigned: cwiiis)

Tracking

Trunk
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
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...
(Assignee)

Comment 1

6 years ago
Created attachment 548739 [details] [diff] [review]
Fix uninitialised PaintState return variable
Attachment #548739 - Flags: review?(matt.woodrow)
(Assignee)

Comment 2

6 years ago
Created attachment 548742 [details] [diff] [review]
Fix uninitialised PaintState return variable

Sorry, another patch was accidentally combined.
Attachment #548739 - Attachment is obsolete: true
Attachment #548739 - Flags: review?(matt.woodrow)
Attachment #548742 - Flags: review?(matt.woodrow)
I think I'd rather add a constructor to set mDidSelfCopy to false, good catch though
(Assignee)

Comment 4

6 years ago
Created attachment 552323 [details] [diff] [review]
Fix uninitialised PaintState return variable by adding a constructor

Updated patch that uses a constructor.
Attachment #548742 - Attachment is obsolete: true
Attachment #548742 - Flags: review?(matt.woodrow)
Attachment #552323 - Flags: review?(matt.woodrow)
(Assignee)

Updated

6 years ago
Assignee: nobody → chrislord.net
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.
Attachment #552323 - Flags: review?(matt.woodrow) → review+
(Assignee)

Comment 6

6 years ago
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.
Attachment #552323 - Attachment is obsolete: true
Attachment #553142 - Flags: review?(matt.woodrow)
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.
Attachment #553142 - Flags: review?(matt.woodrow) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/f345becaf55b
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/f345becaf55b
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.