1125401 - Handle multipart images correctly by replacing ProgressTracker::IsLoading with checks of the correct progress flags

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

Right now ProgressTracker::IsLoading() checks FLAG_LOAD_COMPLETE, which was OK when we reset that flag for every part of a multipart image.

Now that we don't, that's wrong. Different callers of IsLoading() need different things, so I think we should just rip it out and replace all callsites with checks for either FLAG_LOAD_COMPLETE or FLAG_LAST_PART_COMPLETE.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Replace ProgressTracker::IsLoading() with checks of the correct progress flags

Here's the patch.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Thanks for the review!

Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ad3baed1c70

Comment 3

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9ad3baed1c70

Comment 4

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8554016 [details] [diff] [review]
Replace ProgressTracker::IsLoading() with checks of the correct progress flags

Approval Request Comment
[Feature/regressing bug #]: Bug 1112972
[User impact if declined]: MJPEG streams do not stop loading when the tab is closed, wasting user bandwidth and CPU time.
[Describe test coverage new/current, TreeHerder]: On central.
[Risks and why]: Low risk; behavior change is limited to MJPEG streams.
[String/UUID change made/needed]: None.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Seth, does this affect 36 too? thanks

Comment 6

[Seth Fowler [:seth] [:s2h]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #5)
> Seth, does this affect 36 too? thanks

Nope, should only be on 37 and 38. Thanks for the quick approval!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/978caf0fb187

Comment 8

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8554016 [details] [diff] [review]
Replace ProgressTracker::IsLoading() with checks of the correct progress flags

Approval Request Comment
[Feature/regressing bug #]: Unknown.
[User impact if declined]: MJPEG streams and webcams stutter and don't render correctly.
[Describe test coverage new/current, TreeHerder]: Already in Firefox 38 and 37.
[Risks and why]: Low. This is a small bug fix for bug 1112956, which actually fixes the issue.
[String/UUID change made/needed]: None.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/41f8742f7c93