Last Comment Bug 661583 - xhr.response needs to hold on to the cache token when the responseType is blob.
: xhr.response needs to hold on to the cache token when the responseType is blob.
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla6
Assigned To: Masatoshi Kimura [:emk]
Depends on:
Blocks: 649133
  Show dependency treegraph
Reported: 2011-06-02 10:54 PDT by Kyle Huey [:khuey] (
Modified: 2011-08-12 09:14 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (2.69 KB, patch)
2011-06-03 10:07 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Splinter Review
patch v2 (3.00 KB, patch)
2011-06-03 21:02 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Splinter Review
patch v2.1 (3.00 KB, patch)
2011-06-03 21:58 PDT, Masatoshi Kimura [:emk]
jonas: review+
Details | Diff | Splinter Review
patch v3 (2.79 KB, patch)
2011-06-19 11:44 PDT, Masatoshi Kimura [:emk]
VYV03354: review+
Details | Diff | Splinter Review
Reordered patch (2.73 KB, patch)
2011-06-30 13:45 PDT, Kyle Huey [:khuey] (
khuey: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] ( 2011-06-02 10:54:35 PDT
From nsICachingChannel:

123     /**
124      * Get the "file" where the cached data can be found.  This is valid for
125      * as long as a reference to the cache token is held.  This may return
126      * an error if cacheAsFile is false.
127      */
128     readonly attribute nsIFile cacheFile;

If we're not holding the cache token alive, this file might be removed from the disk.  The easiest way forward here is probably to make nsDOMFile able to hold onto an opaque nsISupports pointer, and then give the constructor the cacheToken from the caching channel.
Comment 1 Kyle Huey [:khuey] ( 2011-06-02 10:57:55 PDT
Also, we have to ensure that slices hold onto the cache token as well.
Comment 2 Jonas Sicking (:sicking) PTO Until July 5th 2011-06-02 11:21:45 PDT
Masatoshi: Do you think you'd be able to get to this? I think we should fix this for FF6 or disable the feature.
Comment 3 Masatoshi Kimura [:emk] 2011-06-03 10:07:52 PDT
Created attachment 537172 [details] [diff] [review]

Please apply this on top of the patch of bug 661582.
Comment 4 Masatoshi Kimura [:emk] 2011-06-03 21:02:46 PDT
Created attachment 537305 [details] [diff] [review]
patch v2
Comment 5 Masatoshi Kimura [:emk] 2011-06-03 21:58:32 PDT
Created attachment 537313 [details] [diff] [review]
patch v2.1
Comment 6 Jonas Sicking (:sicking) PTO Until July 5th 2011-06-09 17:42:47 PDT
Comment on attachment 537313 [details] [diff] [review]
patch v2.1

Looks good, but add the new aCacheToken argument to the first constructor instead, as per the review of bug 661582.
Comment 7 Masatoshi Kimura [:emk] 2011-06-19 11:44:43 PDT
Created attachment 540336 [details] [diff] [review]
patch v3

Rebased to the latest bug 661583 patch.
Carrying forward r+.
Comment 8 Kyle Huey [:khuey] ( 2011-06-30 13:45:18 PDT
Created attachment 543237 [details] [diff] [review]
Reordered patch

This is Masatoshi's patch reordered to not depend on the patch in Bug 661582.
Comment 9 Kyle Huey [:khuey] ( 2011-06-30 13:46:58 PDT
Comment on attachment 543237 [details] [diff] [review]
Reordered patch

Drivers, we will want to take this patch for Firefox 6 to ensure that a File object's actual file on disk is not removed from the cache depending on the whims of GC behavior.  I believe the risk of this patch is very low.
Comment 10 christian 2011-06-30 14:36:22 PDT
Comment on attachment 543237 [details] [diff] [review]
Reordered patch

Approved for releases/mozilla-aurora. Please land asap before 2011-07-05 @ 9:00 am PDT.
Comment 12 Matt Brubeck (:mbrubeck) 2011-07-01 12:04:43 PDT
Backed out from mozilla-central during investigation of Android browser-chrome test failures:
Comment 13 Kyle Huey [:khuey] ( 2011-07-01 19:36:41 PDT
Comment 14 Virgil Dicu [:virgil] [QA] 2011-08-12 09:14:03 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20100101 Firefox/6.0

Could you please provide a test case in order to have this issue verified?

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