740841 - if not decoded RasterImage::GetFrame won't use the right decode flags

Status: REOPENED

(Core :: Graphics: ImageLib, defect)

Description

[Timothy Nikkel (:tnikkel)]

Attachment: patch

If you call RasterImage::GetFrame with mDecoded true and decode flags different from mFrameDecodeFlags we never discard and update our decode flags because that code is inside an "if (mDecoded)" check.

The mDecoded check was added by bug 647802. Bug 647802, comment 11 explains that the problem was that mDecoded was false which made CanForciblyDiscard return false. At the time CanForciblyDiscard read:

  return (mDiscardable &&        // ...Enabled at creation time...
          mHasSourceData &&      // ...have the source data...
          mDecoded);             // ...and have something to discard.

Since then bug 672578 changed it to

  return mDiscardable &&      // ...Enabled at creation time...
         mHasSourceData;      // ...have the source data...

So it seems like we don't need the mDecoded check anymore. The same block of code is repeated in several places in the file without the mDecoded check. Does that sound right? Guessing at reviewer choice.

Comment 1

[Joe Drew (not getting mail)]

Comment on attachment 610909 [details] [diff] [review]
patch

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

This actually makes GetFrame *more* like the rest of RasterImage (CopyFrame, etc), which I am all in favour of.

Comment 2

[Joe Drew (not getting mail)]

But please do test the testcases in bug 647802.

Comment 3

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 610909
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=b33249a7e34d
Try run started, revision b33249a7e34d. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=b33249a7e34d

Comment 4

[Mozilla RelEng Bot]

Try run for b33249a7e34d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b33249a7e34d
Results (out of 15 total builds):
    exception: 1
    success: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-b33249a7e34d

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to Joe Drew (:JOEDREW!) from comment #2)
> But please do test the testcases in bug 647802.

I tested http://people.mozilla.org/~bjacob/webgltexture/ on Fennec. It worked fine.

Comment 6

[Timothy Nikkel (:tnikkel)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5c4aedd43a6

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/f5c4aedd43a6

Comment 8

[Timothy Nikkel (:tnikkel)]

I backed this out on inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6da50562fff0

Comment 9

[Marco Bonardo [:mak]]

merged backout https://hg.mozilla.org/mozilla-central/rev/6da50562fff0