695498 - Cleanup WriteToDecoder a little

Status: REOPENED

(Core :: Graphics: ImageLib, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Cleanup WriteToDecoder a little

Removes the unobvious CONTAINER_ENSURE() which makes the fact that we only adjust
mBytesDecoded when we don't have success more obvious. This also takes out a check for mDecoder that is unneeded because of the kungFuDeathGrip.

Comment 1

[Bobby Holley (:bholley)]

Comment on attachment 567865 [details] [diff] [review]
Cleanup WriteToDecoder a little

>+  if (status == NS_OK) {

if (NS_SUCCEEDED(status)) {

>+    // Keep track of the total number of bytes written over the lifetime of the
>+    // decoder

Can you add a period here?

r=bholley with that.

Comment 2

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7aa60d6408b3

[Please can you use the "assign bug to me" checkbox when attaching patches, as it makes the merge much quicker if the assignee has been filled out correctly - thanks :-) ]

Comment 3

[Joe Drew (not getting mail)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #0)
> Created attachment 567865 [details] [diff] [review] [diff] [details] [review]
> Cleanup WriteToDecoder a little
> 
> Removes the unobvious CONTAINER_ENSURE() which makes the fact that we only
> adjust
> mBytesDecoded when we don't have success more obvious. This also takes out a
> check for mDecoder that is unneeded because of the kungFuDeathGrip.

WHOA. That is not the case. See bug 639303 comment 24. This needs to be fixed right now by backout, as it will reintroduce security bugs.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

I have backed this out:
https://hg.mozilla.org/mozilla-central/rev/b0f02951ac46

Comment 5

[Michael Lefevre]

Reopening as fix was backed out. Although if the basis for doing this was wrong I guess this should be resolved another way...