Last Comment Bug 673417 - imgLib cleanups
: imgLib cleanups
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla8
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-22 08:40 PDT by Jeff Muizelaar [:jrmuizel]
Modified: 2011-08-06 05:28 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
mLoadTime has never been used (1.22 KB, patch)
2011-07-22 08:43 PDT, Jeff Muizelaar [:jrmuizel]
joe: review+
Details | Diff | Splinter Review
Kill more PRBools (3.16 KB, patch)
2011-07-22 08:44 PDT, Jeff Muizelaar [:jrmuizel]
joe: review+
dholbert: review-
Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2011-07-22 08:40:59 PDT

    
Comment 1 Jeff Muizelaar [:jrmuizel] 2011-07-22 08:43:16 PDT
Created attachment 547699 [details] [diff] [review]
mLoadTime has never been used

mLoadTime was added by stuart back in 2001. It hasn't as far as I can tell ever been used.
Comment 2 Jeff Muizelaar [:jrmuizel] 2011-07-22 08:44:17 PDT
Created attachment 547701 [details] [diff] [review]
Kill more PRBools
Comment 3 Daniel Holbert [:dholbert] 2011-07-22 11:17:04 PDT
Comment on attachment 547701 [details] [diff] [review]
Kill more PRBools

>diff --git a/modules/libpr0n/src/RasterImage.cpp b/modules/libpr0n/src/RasterImage.cpp
>-PRBool
>+bool
> RasterImage::ShouldAnimate()
> {
>   return Image::ShouldAnimate() && mFrames.Length() >= 2 &&
>          !mAnimationFinished;
[...]
>diff --git a/modules/libpr0n/src/RasterImage.h b/modules/libpr0n/src/RasterImage.h
> protected:
>-  PRBool ShouldAnimate();
>+  bool ShouldAnimate();
> };

ShouldAnimate overrides an inherited method (from 'Image').  You'll need to also change the method in Image & VectorImage, or this will break method-overriding.
Comment 4 Joe Drew (not getting mail) 2011-07-25 13:55:37 PDT
Comment on attachment 547699 [details] [diff] [review]
mLoadTime has never been used

Review of attachment 547699 [details] [diff] [review]:
-----------------------------------------------------------------

I only wish I could r- this
Comment 5 Joe Drew (not getting mail) 2011-07-25 13:55:55 PDT
Comment on attachment 547701 [details] [diff] [review]
Kill more PRBools

Review of attachment 547701 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 6 Daniel Holbert [:dholbert] 2011-07-25 15:32:25 PDT
Comment on attachment 547701 [details] [diff] [review]
Kill more PRBools

Explicitly setting r-, to make sure Comment 3 gets addressed / noticed. :)
Comment 8 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-06 03:49:00 PDT
mwu has some mass boolification planned, so you probably don't need to do your own boolification patches.
Comment 9 Daniel Holbert [:dholbert] 2011-08-06 05:28:06 PDT
(In reply to Daniel Holbert [:dholbert] from comment #6)
> Explicitly setting r-, to make sure Comment 3 gets addressed / noticed. :)

(/me notes that this was indeed addressed in cset d5db87177940.  Thanks! )

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