Last Comment Bug 686453 - Ensure as many cairo surfaces are non-null as possible
: Ensure as many cairo surfaces are non-null as possible
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla10
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 564231
  Show dependency treegraph
 
Reported: 2011-09-13 07:48 PDT by Josh Matthews [:jdm]
Modified: 2011-10-03 08:03 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Create invalid cairo surfaces instead of null pointers when possible. (12.16 KB, patch)
2011-09-13 07:49 PDT, Josh Matthews [:jdm]
jmuizelaar: review+
Details | Diff | Review

Description Josh Matthews [:jdm] 2011-09-13 07:48:02 PDT

    
Comment 1 Josh Matthews [:jdm] 2011-09-13 07:49:55 PDT
Created attachment 559956 [details] [diff] [review]
Create invalid cairo surfaces instead of null pointers when possible.
Comment 2 Jeff Muizelaar [:jrmuizel] 2011-09-23 13:33:13 PDT
Comment on attachment 559956 [details] [diff] [review]
Create invalid cairo surfaces instead of null pointers when possible.

It's not crystal clear that this is an improvement but feels like it's in the right direction.

The added white space changes make me a little sad

>@@ -83,27 +88,29 @@
> {
>     mCGContext = cairo_quartz_surface_get_cg_context (csurf);
>     CGContextRetain (mCGContext);
> 
>     Init(csurf, PR_TRUE);
> }
> 
> gfxQuartzSurface::gfxQuartzSurface(unsigned char *data,
>-                                   const gfxSize& size,
>+                                   const gfxSize& realSize,
>                                    long stride,
>                                    gfxImageFormat format,
>                                    PRBool aForPrinting)

Why realSize?

>-    : mCGContext(nsnull), mSize(size), mForPrinting(aForPrinting)
>+    : mCGContext(nsnull), mSize(realSize), mForPrinting(aForPrinting)
> {
>-    unsigned int width = (unsigned int) floor(size.width);
>-    unsigned int height = (unsigned int) floor(size.height);
>+    gfxIntSize size((unsigned int) floor(realSize.width),
>+                    (unsigned int) floor(realSize.height));
>+    if (!CheckSurfaceSize(size))
>+        MakeInvalid();
> 
>-    if (!CheckSurfaceSize(gfxIntSize(width, height)))
>-        return;
>+    unsigned int width = (unsigned int) floor(mSize.width);
>+    unsigned int height = (unsigned int) floor(mSize.height);

We shouldn't need to floor mSize

> # Animated gifs with a very large canvas, but tiny actual content.
>-asserts(2) load delaytest.html?523528-1.gif # Bug 564231
>+load delaytest.html?523528-1.gif

Yay!
Comment 4 Marco Bonardo [::mak] 2011-10-03 08:03:48 PDT
https://hg.mozilla.org/mozilla-central/rev/02ae1f7047eb

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