Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Implement <canvas>.toBlob

RESOLVED FIXED in Firefox 18

Status

()

Core
DOM
P1
normal
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

({dev-doc-complete})

unspecified
mozilla19
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

Details

(URL)

Attachments

(1 attachment)

We should do this and get it standardized.

Not sure whether or not we need to keep mozGetAsFile.  I think we can afford to just drop it completely.
Assignee: nobody → khuey
Status: NEW → ASSIGNED
So, the comment in the spec has basically

  void toBlob(FileCallback cb, optional DOMString type, any... args);

I'm not sure if it's useful to change the name of the API now, as we'll need to change it at least once more. Waiting until Hixie fleshes this out and implement the spec then seems more logical to me.
Summary: Replace canvas.mozGetAsFile with canvas.mozGetBlob → Implement <canvas>.toBlob
I am currently developing a patch using |mozGetAsFile| for bug 753768 but I believe that it would work just as well with |toBlob|.
Gaia is going to want this to go fast.
blocking-basecamp: --- → ?
Created attachment 671562 [details] [diff] [review]
Patch

This doesn't do off-main-thread encoding, but it does dispatch the response off the event loop to mimic the async behavior.
Attachment #671562 - Flags: review?(bugs)

Comment 5

5 years ago
Comment on attachment 671562 [details] [diff] [review]
Patch

>+// XXXkhuey this should be async, but we're lazy.
You mean off-main-thread?


>+NS_IMETHODIMP
>+nsHTMLCanvasElement::ToBlob(nsIFileCallback* aCallback,
>+                            const nsAString& aType,
>+                            nsIVariant* aParams,
>+                            uint8_t optional_argc)
aParamName


Before using aType, it should be lowercased.


>+{
>+  // do a trust check if this is a write-only canvas
>+  if ((mWriteOnly) &&
No need for () around mWriteOnly

>+      !nsContentUtils::IsCallerTrustedForRead()) {
>+    return NS_ERROR_DOM_SECURITY_ERR;
>+  }
>+  void* imgData = nullptr;
>+  rv = NS_ReadInputStreamToBuffer(stream, &imgData, (uint32_t)imgSize);
I'd prefer C++ cast

>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  // The DOMFile takes ownership of the buffer
>+  nsRefPtr<nsDOMMemoryFile> blob =
>+    new nsDOMMemoryFile(imgData, (uint32_t)imgSize, type);
ditto
Attachment #671562 - Flags: review?(bugs) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/227e87a7c628

Comment 7

5 years ago
this is going to be needed to work around many of the the OOM in gallery, video, and music.  +'ing.
blocking-basecamp: ? → +
Blocks: 798002
(In reply to Doug Turner (:dougt) from comment #7)
> this is going to be needed to work around many of the the OOM in gallery,
> video, and music.  +'ing.

Can you elaborate, or point to bugs?

base-64-encoded URIs are 2.7x larger than blobs, but I don't see how using this would actually solve an underlying problem; if we OOM with data URIs, we'd OOM with this, just after showing something like 3x more images.

Comment 9

5 years ago
we can use mozGetAsFile.  leaving just as a nom.
blocking-basecamp: + → ?
Please don't use mozGetAsFile.  We want to remove that eventually.
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/227e87a7c628
https://hg.mozilla.org/mozilla-central/rev/30395df97c29
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment on attachment 671562 [details] [diff] [review]
Patch

Gaia will want this to be able to use memory more efficiently

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Certain Gaia apps may be written with less supported APIs or may crash after running out of memory more often.
Testing completed (on m-c, etc.): On m-c, has automated tests.
Risk to taking this patch (and alternatives if risky): Low.  This is just an async wrapper around an existing synchronous codepath.
String or UUID changes made by this patch: This changes the IID for nsIDOMHTMLCanvasElement.  I could avoid that if strictly necessary, but I don't think it matters.
Attachment #671562 - Flags: approval-mozilla-aurora?
blocking-basecamp: ? → +
Priority: -- → P1
https://hg.mozilla.org/releases/mozilla-aurora/rev/cbfef8616b63 (I qfolded the 2 patches into 1)
status-firefox18: --- → fixed
status-firefox19: --- → fixed
Flags: in-testsuite+
Attachment #671562 - Flags: approval-mozilla-aurora?
This is great!  For the record, Gaia only uses data urls when for transferring images through activities because the actvities API doesn't support blobs yet.  Everywhere else we use mozGetAsFile.  So toBlob() probably won't help with memory at all, unless mozGetAsFile is really inefficient in some way that I don't know about.

I was hopeful that the fact that is async would improve the speed of thumbnail and album art metadata parsing for the Gallery and Music apps and also help the screenshot code in the window manager app.  But give Kyle's comment #4 above, I'm not sure this will help us with speed either.  Could you elaborate, Kyle?

If there is a speed improvement, then I think we should convert the metadata parsing code in Gallery and Music, and the screenshot code in the window manager to use toBlob(), but leave other mozGetAsFile() calls as is for v1.

For v2, we'll obviously want to convert everything to use the standard method instead of the moz-prefixed one.

cc'ing timdream because he owns the window manager screenshot stuff and should know about this.
No longer blocks: 798002
Blocks: 802647

Updated

5 years ago
Depends on: 802926
Blocks: 803684
Depends on: 809587
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #4)
> This doesn't do off-main-thread encoding

Is there a bug for implementing off-main-thread encoding?
No.
Filed bug 817700.
Depends on: 817700
Depends on: 822190
https://developer.mozilla.org/en-US/Firefox/Releases/19
https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement.toBlob
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.