Last Comment Bug 1148684 - Compact SourceBuffer even if it contains only one chunk
: Compact SourceBuffer even if it contains only one chunk
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Downloads Panel (show other bugs)
: unspecified
: All All
-- normal (vote)
: Firefox 39
Assigned To: Seth Fowler [:seth] [:s2h]
:
: :Paolo Amadini
Mentors:
Depends on:
Blocks: 1120271 1145762
  Show dependency treegraph
 
Reported: 2015-03-27 19:09 PDT by Seth Fowler [:seth] [:s2h]
Modified: 2015-04-03 12:25 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Compact SourceBuffer even if it contains only one chunk. (1.25 KB, patch)
2015-03-27 20:39 PDT, Seth Fowler [:seth] [:s2h]
tnikkel: review+
sledru: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Seth Fowler [:seth] [:s2h] 2015-03-27 19:09:38 PDT
Bug 1120271 introduced compacting support for SourceBuffer in ImageLib. Unfortunately, the code assumes that if there is only one chunk in the SourceBuffer, it doesn't need to be compacted. In cases where we got a false value for Content-Length, though, that's not true. Bug 1145762 is an example of such a situation.

The fix: we should always compact, even if we only have one chunk.
Comment 1 User image Seth Fowler [:seth] [:s2h] 2015-03-27 20:39:45 PDT
Created attachment 8584933 [details] [diff] [review]
Compact SourceBuffer even if it contains only one chunk.

Here's the patch. I've verified that this fixes the issue in bug 1145762. (Even
without the GetContentLength fix in bug 1148682.)
Comment 2 User image Seth Fowler [:seth] [:s2h] 2015-03-27 20:41:14 PDT
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4fbc7048d9cc
Comment 3 User image Seth Fowler [:seth] [:s2h] 2015-03-29 16:12:19 PDT
Comment on attachment 8584933 [details] [diff] [review]
Compact SourceBuffer even if it contains only one chunk.

Oops, just realized I didn't request review on this.
Comment 4 User image Seth Fowler [:seth] [:s2h] 2015-03-29 16:26:21 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f64b5c0ee649
Comment 5 User image Carsten Book [:Tomcat] 2015-03-30 03:10:29 PDT
https://hg.mozilla.org/mozilla-central/rev/f64b5c0ee649
Comment 6 User image Seth Fowler [:seth] [:s2h] 2015-03-30 14:30:52 PDT
Comment on attachment 8584933 [details] [diff] [review]
Compact SourceBuffer even if it contains only one chunk.

Approval Request Comment
[Feature/regressing bug #]: Bug 1120271 introduced the bug. This fixes bug 1145762.
[User impact if declined]: Unnecessary memory consumption, resulting in OOM in some cases.
[Describe test coverage new/current, TreeHerder]: On m-c, and I believe now m-a.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.
Comment 7 User image Sylvestre Ledru [:sylvestre] 2015-04-02 06:16:32 PDT
Comment on attachment 8584933 [details] [diff] [review]
Compact SourceBuffer even if it contains only one chunk.

Regressions are unlikely, taking it.
should be in 38 beta 2
Comment 8 User image Ryan VanderMeulen [:RyanVM] 2015-04-03 12:25:04 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/ca8eaf3366e5

Note You need to log in before you can comment on or make changes to this bug.