Closed Bug 840841 Opened 7 years ago Closed 7 years ago

Remove mObserver from RasterImage and VectorImage

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: seth, Assigned: seth)

Details

Attachments

(1 file, 1 obsolete file)

It doesn't seem like the mObserver member that RasterImage and VectorImage have adds any value. They can always get to the only observer they ever get passed - an imgStatusTrackerObserver - via mStatusTracker->GetDecoderObserver(). Also, mObserver is a weak pointer, but we know we can never lose an imgStatusTrackerObserver as long as we hold a strong reference to the corresponding imgStatusTracker. In the interest of reduced memory consumption and complexity it makes sense to eliminate mObserver.
Proposed patch.
Attachment #713256 - Flags: review?(joe)
Attachment #713256 - Flags: review?(joe) → review+
I'm broadly in favour of this, though it's going to conflict with my own changes when I try to rebase on top if it :)
Ack, stupid bzexport! Or possibly stupid me. Regardless, one of these got posted to the wrong bug.
Attachment #713256 - Attachment is obsolete: true
Comment on attachment 713247 [details] [diff] [review]
Remove mObserver from RasterImage and VectorImage.

Try looks good, but rerequesting review because I want to be sure you looked at the right patch. =) Sorry for the mixup.
Attachment #713247 - Flags: review?(joe)
Comment on attachment 713247 [details] [diff] [review]
Remove mObserver from RasterImage and VectorImage.

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

::: image/src/RasterImage.cpp
@@ +2548,5 @@
>    CONTAINER_ENSURE_TRUE(type != eDecoderType_unknown, NS_IMAGELIB_ERROR_NO_DECODER);
>  
>    // Instantiate the appropriate decoder
> +  imgDecoderObserver* observer = mStatusTracker ? mStatusTracker->GetDecoderObserver()
> +                                                : nullptr;

Are there actually times when RasterImages have no status tracker?
Attachment #713247 - Flags: review?(joe) → review+
Thanks for the review, Joe!

(In reply to Joe Drew (:JOEDREW! \o/) from comment #6)
> Are there actually times when RasterImages have no status tracker?

Yeah - ImageFactory::CreateAnonymousImage will create an image with no status tracker. This is called from imgTools::DecodeImageData.
Everything looks OK here. Requesting checkin.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/78d86cf0671b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.