Closed Bug 772434 Opened 8 years ago Closed 8 years ago

Blob support for Zip file contents

Categories

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

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: overholt, Assigned: baku)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, Whiteboard: [games:p1])

Attachments

(1 file, 4 obsolete files)

Zip files stored as blobs should allow for access to files contained within them.  These files would be interacted with as blobs themselves.  Jonas also suggests being able to create a zip of a collection of blobs as an interesting use case.
Spec please.
Blocks: gecko-games
Whiteboard: [games:p1]
This is what I wrote yesterday. Maybe it's a good starting point for a more technical discussion.

2 points:

. consider it as work-in-progress
. feel free to say: we need something completely different

*Goals*

Provide the ability to read the list of files contained in an archive file and to have access to them as Blob objects.

*Status*

https://bugzilla.mozilla.org/show_bug.cgi?id=772434

*Example*

function startRead() {

  var file = document.getElementById('file').files[0];

  // Let's assume that this new Archive API supports serveral archive formats (this code is bad...)
  var formatSupported = [ '.zip', '.tar.gz', '.tgz', '.gz', '.bz2', '.jar' /* ... */ ];
  for (var i = 0; i < formatSupported.length; ++i) {
    if (file.indexOf(formatSupported[i]) != -1) {
      openArchive(file);
      return;
    }
  }

  alert("This archive file is not supported");
}

function openArchive(file) {
  // The ArchiveReader object works with Blob objects:
  var archiveReader = new ArchiveReader(file);

  // Any request is asyncronous:
  var handler = archiveReader.getFilenames();
  handler.onsuccess = success;
  handler.onerror = errorHandler;

  // Multiple requests can run at the same time:
  var handler2 = archiveReader.getFile("images/background.png");
  handler2.onsuccess = fileSuccess;
  handler2.onerror = errorHandler;
}

// The getFilenames handler receives a DOMString[] as result:
function success() {
  // this.result is a DOMString[]:
  for (var i = 0; i < this.result.length; ++i) {
    // ...
  }
}

// The GetFile handler receives a File/Blob object (and it can be used with FileReader API):
function fileSuccess() {
  // var reader = FileReader();
  // reader.readAsText(this.result, ...);
}

function errorHandler(evt) {
  if (evt.error.name == "NotReadbleErr") {
    // ...
  } else {
    // TODO: something else
  }
}

*API*

This is a draft of the API written in WebIDL:

interface ArchiveRequest : DOMRequest
{
  void abort();

  [TreatNonCallableAsNull] attribute Function? onprogress;
}

// Asyncronous API:

[Constructor(Blob blob)]
interface ArchiveReader
{
  // async read methods
  ArchiveRequest getFilenames();
  ArchiveRequest getFile(DOMString filename);
};

// Syncronous API:

[Constructor(Blob blob)]
interface ArchiveReaderSync
{
  DOMString[] getFilenames();
  Blob getFile(DOMString filename);
};
Attached patch first implementation - backup (obsolete) — Splinter Review
I'm just using bugzilla as a backup system for my code.
Attachment #643597 - Attachment is obsolete: true
Attachment #644705 - Flags: review?(jst)
Keywords: dev-doc-needed
code indented.
Assignee: nobody → amarchesini
Attachment #644705 - Attachment is obsolete: true
Attachment #644705 - Flags: review?(jst)
Attachment #646344 - Flags: review?(jst)
Comment on attachment 646344 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch

- In nsDOMArchiveRequest::GetFilenamesResult():

+  JS_FreezeObject(aCx, array);

We should check the return value here and throw an error if this call fails (returns false).

r=jst with that fixed!
Attachment #646344 - Flags: review?(jst) → review+
Comment on attachment 646344 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch

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

You can drop the 'ns' and 'nsDOM' prefixes on everything in the file namespace.

nsDOMArchiveZipFile should inherit from nsDOMFileCC, not nsDOMFile.

There's also a problem here.  You can grab an nsDOMArchiveZipFile and postMessage it to a worker thread, and which point Gecko will abort because we'll try to AddRef a cycle collected object off the main thread.  janv's FileHandle stuff has the same problem.  So somebody needs to figure out how that's going to work ...
Attachment #646344 - Flags: review-
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #7)
> Comment on attachment 646344 [details] [diff] [review]
[...]
> There's also a problem here.  You can grab an nsDOMArchiveZipFile and
> postMessage it to a worker thread, and which point Gecko will abort because
> we'll try to AddRef a cycle collected object off the main thread.  janv's
> FileHandle stuff has the same problem.  So somebody needs to figure out how
> that's going to work ...

Did Jan's FileHandle stuff already land? If so I would say that this belongs in a followup bug, and it should IMO not block this patch from landing.
Yes, it already landed.  I'll take that to a followup, but I'd still like to see the inheritance change and the nsDOM prefixes dropped before landing.
objects renamed + nsDOMFileCC inherited.
I sent the patch to try server just to be sure it compiles and passes the tests on windows, android & C too (but I don't see any reason why it should fail).
Attachment #646344 - Attachment is obsolete: true
Attachment #646444 - Flags: review?(khuey)
Attachment #646444 - Flags: review?(jst)
Comment on attachment 646444 [details] [diff] [review]
Bug 772434 - Blob support for Zip file contents, r=jst

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

If all you did is change the names and the class it inherits from, I don't think this needs another review.
Attachment #646444 - Flags: review?(khuey)
Attachment #646444 - Flags: review?(jst)
I went to land this but decided to build it locally first, and turns out this doesn't build when applied to a current trunk build. NS_OUTPARAM needs to be removed (was removed on trunk), and ArchiveRequest.cpp doesn't compile due to not knowing what CCParticipantVTable is. Likely fallout from the last inheritance changes here.
Attachment #646444 - Attachment is obsolete: true
Attachment #646699 - Flags: checkin?
Would be great if someone could write a hacks.mozilla.org article about this API.
https://hg.mozilla.org/mozilla-central/rev/5bff8785ab1b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Depends on: 781155
Depends on: 781425
Attachment #646699 - Flags: checkin?
I believe this is non-standard, isn't it? Does it build on any standardized spec?
Depends on: 795930
See also https://bugzilla.mozilla.org/show_bug.cgi?id=681967, not sure if this one was a follow-up or not but there might be insightful stuff in the other thread.
Where is this feature with regards to a spec and web standards?
(In reply to Martin Best (:mbest) from comment #20)
> Where is this feature with regards to a spec and web standards?

Nowhere.
Did we ship this to the web?
Yes, but it's disabled by default. Bug 795930
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.