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

REOPENED
Assigned to

Status

()

REOPENED
7 years ago
6 years ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

(Depends on: 1 bug)

Trunk
mozilla14
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 610909 [details] [diff] [review]
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.
Attachment #610909 - Flags: review?(joe)
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.
Attachment #610909 - Flags: review?(joe) → review+
But please do test the testcases in bug 647802.
Whiteboard: [autoland-try]

Updated

7 years ago
Whiteboard: [autoland-try] → [autoland-in-queue]

Comment 3

7 years ago
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

7 years ago
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/autolanduser@mozilla.com-b33249a7e34d

Updated

7 years ago
Whiteboard: [autoland-in-queue]
(Assignee)

Comment 5

7 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/f5c4aedd43a6
Assignee: nobody → tnikkel
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Depends on: 742081
(Assignee)

Updated

7 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
You need to log in before you can comment on or make changes to this bug.