Closed Bug 661583 Opened 9 years ago Closed 9 years ago

xhr.response needs to hold on to the cache token when the responseType is blob.

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla6
Tracking Status
firefox6 + fixed
firefox7 --- affected

People

(Reporter: khuey, Assigned: emk)

References

Details

Attachments

(2 files, 3 obsolete files)

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.
Also, we have to ensure that slices hold onto the cache token as well.
Masatoshi: Do you think you'd be able to get to this? I think we should fix this for FF6 or disable the feature.
Attached patch patch (obsolete) — Splinter Review
Please apply this on top of the patch of bug 661582.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #537172 - Flags: review?(jonas)
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #537172 - Attachment is obsolete: true
Attachment #537172 - Flags: review?(jonas)
Attachment #537305 - Flags: review?(jonas)
Attached patch patch v2.1 (obsolete) — Splinter Review
Attachment #537305 - Attachment is obsolete: true
Attachment #537305 - Flags: review?(jonas)
Attachment #537313 - Flags: review?(jonas)
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.
Attachment #537313 - Flags: review?(jonas) → review+
Attached patch patch v3Splinter Review
Rebased to the latest bug 661583 patch.
Carrying forward r+.
Attachment #537313 - Attachment is obsolete: true
Attachment #540336 - Flags: review+
Attached patch Reordered patchSplinter Review
This is Masatoshi's patch reordered to not depend on the patch in Bug 661582.
Attachment #543237 - Flags: review+
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.
Attachment #543237 - Flags: approval-mozilla-aurora?
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.
Attachment #543237 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/mozilla-central/rev/c3d3292fbd1e
http://hg.mozilla.org/releases/mozilla-aurora/rev/c660b06ec3b3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Backed out from mozilla-central during investigation of Android browser-chrome test failures:
http://hg.mozilla.org/mozilla-central/rev/00bb08972e46
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://hg.mozilla.org/mozilla-central/rev/8c8d3b22ea79
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
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?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.