Last Comment Bug 772434 - Blob support for Zip file contents
: Blob support for Zip file contents
Status: RESOLVED FIXED
[games:p1]
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86_64 Linux
: -- normal with 1 vote (vote)
: mozilla17
Assigned To: Andrea Marchesini [:baku]
:
Mentors:
Depends on: 778023 781155 781425 793434 795930 809567
Blocks: gecko-games
  Show dependency treegraph
 
Reported: 2012-07-10 07:24 PDT by Andrew Overholt [:overholt] (back Aug 31)
Modified: 2013-05-02 09:31 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
first implementation - backup (58.41 KB, patch)
2012-07-18 14:40 PDT, Andrea Marchesini [:baku]
no flags Details | Diff | Splinter Review
Bug 772434, Blob support for Zip file contents - patch (58.78 KB, patch)
2012-07-21 18:56 PDT, Andrea Marchesini [:baku]
no flags Details | Diff | Splinter Review
Bug 772434, Blob support for Zip file contents - patch (58.00 KB, patch)
2012-07-26 14:30 PDT, Andrea Marchesini [:baku]
jst: review+
khuey: review-
Details | Diff | Splinter Review
Bug 772434 - Blob support for Zip file contents, r=jst (56.91 KB, patch)
2012-07-26 19:12 PDT, Andrea Marchesini [:baku]
no flags Details | Diff | Splinter Review
Bug 772434 - Blob support for Zip file contents, r=jst (56.90 KB, patch)
2012-07-27 13:43 PDT, Andrea Marchesini [:baku]
no flags Details | Diff | Splinter Review

Description Andrew Overholt [:overholt] (back Aug 31) 2012-07-10 07:24:26 PDT
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.
Comment 1 :Ms2ger (⌚ UTC+1/+2) 2012-07-10 07:25:52 PDT
Spec please.
Comment 2 Andrea Marchesini [:baku] 2012-07-11 13:15:45 PDT
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);
};
Comment 3 Andrea Marchesini [:baku] 2012-07-18 14:40:45 PDT
Created attachment 643597 [details] [diff] [review]
first implementation - backup

I'm just using bugzilla as a backup system for my code.
Comment 4 Andrea Marchesini [:baku] 2012-07-21 18:56:00 PDT
Created attachment 644705 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch
Comment 5 Andrea Marchesini [:baku] 2012-07-26 14:30:58 PDT
Created attachment 646344 [details] [diff] [review]
Bug 772434, Blob support for Zip file contents - patch

code indented.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2012-07-26 18:09:40 PDT
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!
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-26 18:28:18 PDT
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 ...
Comment 8 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-26 18:29:02 PDT
See comment 7.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2012-07-26 18:36:51 PDT
(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.
Comment 10 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-26 18:44:40 PDT
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.
Comment 11 Andrea Marchesini [:baku] 2012-07-26 19:12:55 PDT
Created attachment 646444 [details] [diff] [review]
Bug 772434 - Blob support for Zip file contents, r=jst

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).
Comment 12 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-26 19:14:41 PDT
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.
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2012-07-27 00:23:42 PDT
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.
Comment 14 Andrea Marchesini [:baku] 2012-07-27 13:43:26 PDT
Created attachment 646699 [details] [diff] [review]
Bug 772434 - Blob support for Zip file contents, r=jst
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2012-07-27 17:22:32 PDT
Landed on inbound!

https://hg.mozilla.org/integration/mozilla-inbound/rev/5bff8785ab1b
Comment 16 Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-07-28 09:32:53 PDT
Would be great if someone could write a hacks.mozilla.org article about this API.
Comment 17 Ryan VanderMeulen [:RyanVM] 2012-07-28 18:34:50 PDT
https://hg.mozilla.org/mozilla-central/rev/5bff8785ab1b
Comment 18 Florian Bender 2012-09-04 01:50:49 PDT
I believe this is non-standard, isn't it? Does it build on any standardized spec?
Comment 19 Paul Bakaus 2012-10-15 02:10:29 PDT
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.
Comment 20 Martin Best (:mbest) 2012-11-19 08:38:41 PST
Where is this feature with regards to a spec and web standards?
Comment 21 :Ms2ger (⌚ UTC+1/+2) 2012-11-19 08:47:51 PST
(In reply to Martin Best (:mbest) from comment #20)
> Where is this feature with regards to a spec and web standards?

Nowhere.
Comment 22 Ted Mielczarek [:ted.mielczarek] 2013-05-02 08:56:24 PDT
Did we ship this to the web?
Comment 23 Andrea Marchesini [:baku] 2013-05-02 09:31:49 PDT
Yes, but it's disabled by default. Bug 795930

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