1312410 - Fetch should reuse XHR's 'store large blobs in temp files' -setup

Status: RESOLVED FIXED

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

Description

[Olli Pettay [:smaug][[e-mail]]]

See bug 1202006

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

It really feels like we should do this for all blobs regardless of how they are produced.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

maybe.  If we are keeping the data anyhow alive somewhere else, no need to create a copy of it as a tmp file.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Olli Pettay [:smaug] from comment #2)
> maybe.  If we are keeping the data anyhow alive somewhere else, no need to
> create a copy of it as a tmp file.

Are you going to persist to disk when that last other thing gets GC'd?  That would be fine.  I think, though, just putting large things in a temp file immediately would be simpler and have little downside.

But we should be consistent that "large blobs are file backed" so devs can use them for seeking access to huge things without worrying about OOMs.  Right now they have to memorize which magic calls happen to do that file-backed behavior and from the folks I've talked to know one really understands it.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: part 1 - MutableBlobStreamListener

Comment 5

[Andrea Marchesini [:baku]]

Attachment: part 2 - tests

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8804148 [details] [diff] [review]
part 1 - MutableBlobStreamListener

I heard a rumor that qDot could do some reviews :)
Want to take a look a this? If not, put it back to my queue.

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8804148 [details] [diff] [review]
part 1 - MutableBlobStreamListener

Review of attachment 8804148 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM (after much other code digging to see how this worked with XHR heh), and glad I caught this review as it could be interesting to route this to encrypted disk storage once we get that done for IDB blobs, instead of just going directly to memory in PB mode.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8044e281e67c
Fetch API should use MutableBlobStorage to store big Blobs on disk, r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/18e1ee2de339
Tests for Fetch API + MutableBlobStorage, r=qdot

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8044e281e67c
https://hg.mozilla.org/mozilla-central/rev/18e1ee2de339