132319 - Browser crashes when machine is low on memory M1RC2 [@ imgContainer::FillWithColor]

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Jérémie Van Renterghem]

This happend to me few times. Each time I had a system warning message telling
that the system was low on memory. Immediately after, browser crashes, even if
it is just open, but not busy. Software version is Mozilla M0.9.9.

Comment 1

[Adam Hauner]

Do you have talkback ID?

Comment 2

[Jérémie Van Renterghem]

if this can help, here are Talkback Incident IDs related to the reported bug :
TB4261389K, TB4253418E, TB4215708G
Hope it helps.

Comment 3

[Adam Hauner]

Stephen, should I ask you for TB4261389K, TB4253418E, TB4215708G?

Comment 4

[[e-mail] (gone - use [e-mail] instead)]

imgContainer::FillWithColor
[d:\builds\seamonkey\mozilla\modules\libpr0n\src\imgContainer.cpp, line 659]
imgContainer::DoComposite
[d:\builds\seamonkey\mozilla\modules\libpr0n\src\imgContainer.cpp, line 532]
imgContainer::Notify
[d:\builds\seamonkey\mozilla\modules\libpr0n\src\imgContainer.cpp, line 458]
nsTimerImpl::Process [d:\builds\seamonkey\mozilla\xpcom\threads\nsTimerImpl.cpp,
line 342]
handleMyEvent [d:\builds\seamonkey\mozilla\xpcom\threads\nsTimerImpl.cpp, line 381]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 524]
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line
1072] 

Comment 5

[Adam Hauner]

Jeremie: Are you able to reproduce this bug with Mozilla Release Candidate 1? 
Any newer talkback?
http://ftp.mozilla.org/pub/mozilla/releases/mozilla1.0rc1/

Comment 6

[Matthias Versen [:Matti]]

why not reassign it to the correct component ?

-> Imagelib (new based on the stack)

Comment 7

[Jérémie Van Renterghem]

I've installed Moz1.0rc1 (with talkback). I'll let you know if new crashes occur.

Comment 8

[Jérémie Van Renterghem]

OK, I think I've got one. Have a look at talkback ID TB5633958K.

Comment 9

[ArronM (:paper)]

Attachment: Cleanup of imgContainer::FillWithColor

What a good opportunity to clean up FillWithColor()

Changes in FillWithColor with this patch:

- New memory allocation reduced to one row of image (bpr).  FillWithColor used
to allocate enough memory to hold the whole image, even though it only used one
row of that memory.

- Validation of memory allocation.  If failed, function exits.

- Check if R, G, & B are the same.  If they are, do a memset and skip memory
allocation alltogether.

- Got rid of some duplicate variables.

Comment 10

[Stuart Parmenter]

tor can you sr=?

Comment 11

[Stuart Parmenter]

Comment on attachment 81547 [details] [diff] [review]
Cleanup of imgContainer::FillWithColor

r=pavlov.

style nitpicks for libpr0n:
+	 }
+	 else	    
+	 {	  

should be:
+	 } else {

Comment 12

[tor]

Comment on attachment 81547 [details] [diff] [review]
Cleanup of imgContainer::FillWithColor

sr=tor

minor nit: rename "foo" to "tmpRow" or something else that indicates its
purpose.

Comment 13

[ArronM (:paper)]

Attachment: Cleanup of imgContainer::FillWithColor

changes as per reviews, plus I removed the trailing spaces.

Comment 14

[Stuart Parmenter]

Comment on attachment 81634 [details] [diff] [review]
Cleanup of imgContainer::FillWithColor

carrying over r= and sr=

Comment 15

[Stuart Parmenter]

checked in fix for [e-mail] to the trunk.

Comment 16

[greer]

Attachment: Crash comments and count

Pavlov, these crashes are still happening in the release candidates. This fix
has benn on the Trunk for two weeks and has stopped the crshes there. We need
to get this fix on the branch.

Comment 17

[Paul Wyskoczka]

Terri, can you verify this on the trunk.  Thx.

Comment 18

[Terri Preston]

I have tried to crash this using win XP trunk build 2002052208, I followed
several of the crash comments and this seems to be fixed

Comment 19

[Gagan]

Seems like it has been verified on trunk. approving on adt behalf --paul really:)

Comment 20

[ArronM (:paper)]

Bug 143333 contains a patch for a regression this bug fix caused.  It also has
been checked into the trunk and confirmed fixed.  You'll probably want it on the
branch as well.

Comment 21

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) for approval to checkin to the 1.0 branch, pending
Drivers' approval and the checkin of the fix for Bug 143333.  After, checking
in, please add the fixed1.0 keyword.

Comment 22

[Stuart Parmenter]

i have both fixes in my branch tree ready to check in as soon as I get drivers
approval for them

Comment 23

[scottputterman]

adding the adt1.0.0+ that Jaime mentioned in a previous comment.

Comment 24

[scottputterman]

changing to adt1.0.1+ for checkin to the 1.0 branch.  Please get drivers
approval before checking in.

Comment 25

[Paul Wyskoczka]

adding mozilla1.0.1 nomination.

Comment 26

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 27

[Terri Preston]

has this been checked into branch?

Comment 28

[Blake Ross]

Fixed on branch.

Comment 29

[ArronM (:paper)]

Need Bug 143333 checked into branch.  Otherwise Linux people won't be happy.

Comment 30

[Terri Preston]

I will wait to verify for branch until 143333 is fixed (I just removed branch
keyword 'cause the branch is not fixed for that bug)