Closed Bug 1259513 Opened 4 years ago Closed 3 years ago

Don't use the gfxContext constructor directly

Categories

(Core :: Graphics, defect)

Unspecified
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: milan, Assigned: milan)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

gfxContext needs to have a valid draw target to be constructed.  Don't attempt to make one of those if the target is invalid, and make it obvious to the callers they will not get a gfxContext if the draw target is invalid, by removing direct access to the constructor, and instead having a utility function that can return failure.
Assignee: nobody → milan
Whiteboard: [gfx-noted]
Attachment #8734451 - Flags: feedback?(jmuizelaar)
Attachment #8734451 - Flags: feedback?(bas)
Would it make more sense to just have the constructor crash and make the users check the drawtarget before constructing the gfxContext?
Flags: needinfo?(milan)
That's what we do today, and it hasn't worked.  I'm hoping that having an explicit method that looks like it can fail, instead of a constructor, which people assume does not, would help this.
Flags: needinfo?(milan)
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42243/diff/1-2/
Attachment #8734451 - Flags: feedback?(jmuizelaar)
Attachment #8734451 - Flags: feedback?(bas)
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

Critical note if the arguments aren't checked, just to make some more noise and a few places where the calling code wasn't consistent.
Attachment #8734451 - Flags: feedback?(jmuizelaar)
Attachment #8734451 - Flags: feedback?(bas)
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

https://reviewboard.mozilla.org/r/42243/#review38841

With the caveat that we must now make sure that we check the result of gfxContext::ForDrawTarget when we use it, r+.
Attachment #8734451 - Flags: review?(lsalzman) → review+
Attachment #8734451 - Flags: feedback?(bas) → feedback+
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

https://reviewboard.mozilla.org/r/42243/#review39213
Attachment #8734451 - Flags: review+
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42243/diff/2-3/
Attachment #8734451 - Flags: feedback?(jmuizelaar)
Attachment #8734451 - Flags: feedback+
(In reply to Milan Sreckovic [:milan] from comment #8)
> Comment on attachment 8734451 [details]
> MozReview Request: Bug 1259513: Make gfxContext constructor private, use a
> utility function that can return nullptr. r?lsalzman
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/42243/diff/2-3/

This is just a rebase.
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42243/diff/3-4/
Comment on attachment 8734451 [details]
MozReview Request: Bug 1259513: Make gfxContext constructor private, use a utility function that can return nullptr. r?lsalzman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42243/diff/4-5/
https://hg.mozilla.org/mozilla-central/rev/da45b1e0d42a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.