463307 - "ASSERTION: zero source size --- fix caller"

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: zero source size --- fix caller: '!aSubimage.IsEmpty()', file /Users/jruderman/central/gfx/src/thebes/nsThebesImage.cpp, line 448

This assertion was added in bug 458487.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Convert to gfxFloat a bit earlier to avoid an overflow in aPt.y - aDest.y

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 349342 [details] [diff] [review]
fix

The reason this patch fixes things is that it avoids integer overflow, right?

These should be const methods:
+    gfxPoint TopLeft() { return pos; }
+    gfxPoint BottomRight() { return gfxPoint(XMost(), YMost()); }

You might also consider making some things like devPixelDest const as well.

r+sr=dbaron

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #2)
> (From update of attachment 349342 [details] [diff] [review])
> The reason this patch fixes things is that it avoids integer overflow, right?

Yes

> These should be const methods:
> +    gfxPoint TopLeft() { return pos; }
> +    gfxPoint BottomRight() { return gfxPoint(XMost(), YMost()); }
> 
> You might also consider making some things like devPixelDest const as well.

OK

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 349342 [details] [diff] [review]
fix

Simple safe patch to fix an assertion

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed to trunk as 3f1286ff2118

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 349342 [details] [diff] [review]
fix

a191=beltzner

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c3a62102650a

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And:
http://hg.mozilla.org/mozilla-central/rev/9335c05bb78b
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a25ae170ffd0

(broke the tree a second time, then I found the bustage fix)

Comment 9

[Aakash Desai [:aakashd]]

I'm seeing the following assertion when running the test case on 1.9.1 branch with the following build Id:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre ID:20090608122057

This isn't seen on mozilla-central (with the same date) though.

Comment 10

[Aakash Desai [:aakashd]]

I'm seeing the following assertion when running the test case on 1.9.1 branch with the following build Id:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre ID:20090608122057

###!!! ASSERTION: We must be allowed to sample *some* source pixels!: '!sourceRect.Intersect(subimage).IsEmpty()', file /Users/adesai/debug/shiretoko/mozilla-1.9.1/gfx/src/thebes/nsThebesImage.cpp, line 497
###!!! ASSERTION: We must be allowed to sample *some* source pixels!: '!sourceRect.Intersect(subimage).IsEmpty()', file /Users/adesai/debug/shiretoko/mozilla-1.9.1/gfx/src/thebes/nsThebesImage.cpp, line 497



This isn't seen on mozilla-central (with the same date) though.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #10)
> ###!!! ASSERTION: We must be allowed to sample *some* source pixels!:
> '!sourceRect.Intersect(subimage).IsEmpty()', file
> /Users/adesai/debug/shiretoko/mozilla-1.9.1/gfx/src/thebes/nsThebesImage.cpp,
> line 497

This has been moved to bug 497214.

Verified fixed with builds on trunk and 1.9.1:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090609 Minefield/3.6a1pre ID:20090609212135

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090609 Shiretoko/3.5pre ID:20090609224410