blob size is wrong in open activity after bluetooth image transfer

RESOLVED FIXED in Firefox 24

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: djf, Assigned: baku)

Tracking

unspecified
mozilla24
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

(Whiteboard: inarirun2)

Attachments

(2 attachments, 2 obsolete attachments)

When I receive an image by bluetooth and open it, the open activity in apps/gallery/js/open.js is called to display it. That code wants to check the size of the file before attempting to open it to prevent OOM errors on very large images.

But the blob.size property is always 18446744073709552000 so all the images appear to be very, very large.

This seems like a gecko or activiies issue, so I probably have the component wrong.

Note that this blocks a bug that is tef+
Marking this as blocking bug 848916.

Cc'ing Fabrice and Ben because they always seem to know what to do when blobs go bad.  Also Evelyn and Ian in case this bug is specific to bluetooth.
Blocks: 848916
Note that the received blob also has an empty type field. That might or might not be the same bug, however.

Needinfo fabrice, at his request.
Flags: needinfo?(fabrice)
I've looked at this some more.  I'm using the nightly b2g-18 build from today.

When I do a share activity from the Gallery to the Bluetooth app (both OOP) it works fine, and the receiving app gets a blob with a name, size and type.

But when the system app receives a file by bluetooth, and then uses an open activity to display the file, the same thing fails.  System initiates the activity, passing a file object. Gallery handles the activity, but the blob it receives does not have a name or a type, and its size is always 0x10000000000000180. That's more than 64 bits so something pretty wacky is happening with the size conversion.

The blob itself seems to contain the right data. It just the properties that are broken, and apparently only when transfering from the system app to another app.

I have confirmed when the system app initiates the activity, the file has a valid name, size and type.
Given that the blob seems to work as a valid file, I was hoping I could workaround the lack of size with:

blob = new Blob([blob]);

But I get the same size when I do that.

Next I tried new FileReader().readAsArrayBuffer(blob), thinking I could check the size of the array buffer it created. But that just crashed the app.  (OOM maybe since the reported size is so big?)
That blob size, 18446744073709552000, is how JavaScript prints Math.pow(2,64). Its the closest JS integer approximation.  It would also be the closest approximation to 2**64 - 1, which would be the value -1 (or any small negative number) in an signed int64.
Talking with David on IRC, the system app gets the file from bluetooth, saves it to device storage, and then shares this file-backed blob. The size is correct in the system app, but gets corrupted afterwards.

Since David has a workaround for now, we are not blocked but more investigation is needed.
Flags: needinfo?(fabrice)
(In reply to David Flanagan [:djf] from comment #5)
> That blob size, 18446744073709552000, is how JavaScript prints
> Math.pow(2,64). Its the closest JS integer approximation.  It would also be
> the closest approximation to 2**64 - 1, which would be the value -1 (or any
> small negative number) in an signed int64.

When a nsIDOMFile is created, the size is set to UINT64_MAX, so that IsSizeUnknown() returns true. Then, just the first time GetSize() is called, if (isSizeUnknown()), the real size is read from the file.

I think the reason why this bug exists, is because somehow the serialization of blobs process-to-process is broken, or it doesn't share size and type.

This happens only -i'm not 100% sure about this yet- with MozActivity.

/me still debugging it.
Blocks: 848723
Posted patch patch (obsolete) — Splinter Review
The activity starts from the system app that runs in the main process.
With this patch the blob is created fully (size, date and content type) as it is in the DeviceStorageRequestParent.cpp
Attachment #747331 - Flags: review?(doug.turner)
Assignee: nobody → amarchesini
Blocks: 850542
Blocks: 870731
Posted patch patch (obsolete) — Splinter Review
This patch fixes the cursor() and the get() requests.
Attachment #747331 - Attachment is obsolete: true
Attachment #747331 - Flags: review?(doug.turner)
Attachment #747904 - Flags: review?(doug.turner)
Comment on attachment 747904 [details] [diff] [review]
patch

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

shouldn't we do this work in the blob?  lets get bent's review.

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +712,5 @@
> +
> +  nsAutoCString mimeType;
> +  nsCOMPtr<nsIMIMEService> mimeService =
> +    do_GetService(NS_MIMESERVICE_CONTRACTID);
> +  if (mimeService) {

return early if !mimeService

@@ +720,5 @@
> +      return rv;
> +    }
> +  }
> +
> +  mMimeType = NS_ConvertUTF8toUTF16(mimeType);

probably best to set this to something unknown if this function fails.
Attachment #747904 - Flags: review?(doug.turner) → review?(bent.mozilla)
Nominating for 1.1 -- this blocks a leo+ QE1 bug.
blocking-b2g: --- → leo?
Comment on attachment 747904 [details] [diff] [review]
patch

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

Looks good to me!
Attachment #747904 - Flags: review?(bent.mozilla) → review+
Component: Gaia::Bluetooth File Transfer → DOM: Device Interfaces
OS: Mac OS X → All
Product: Boot2Gecko → Core
Hardware: x86 → All
Duplicate of this bug: 848723
It blocks the bug 850542. So blocking
blocking-b2g: leo? → leo+
Hi baku,

It seems that the patch has been ready for a few days. Shall we?
Flags: needinfo?(amarchesini)
(In reply to Eric Chou [:ericchou] [:echou] from comment #15)
> Hi baku,
> 
> It seems that the patch has been ready for a few days. Shall we?

It's almost green on try. There is still an orange related to certificates. I hope to see/make it green for today. Then, yes. We can land it.
Flags: needinfo?(amarchesini)
(In reply to Andrea Marchesini (:baku) from comment #16)
> (In reply to Eric Chou [:ericchou] [:echou] from comment #15)
> > Hi baku,
> > 
> > It seems that the patch has been ready for a few days. Shall we?
> 
> It's almost green on try. There is still an orange related to certificates.
> I hope to see/make it green for today. Then, yes. We can land it.

Thank you, baku.
Posted patch m-c patchSplinter Review
Attachment #747904 - Attachment is obsolete: true
Attachment #751011 - Flags: review+
Posted patch b2g18 patchSplinter Review
Attachment #751029 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/27c73f18702b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Duplicate of this bug: 850542
Whiteboard: inarirun2
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap-
QA Contact: amiller
Depends on: 877482
You need to log in before you can comment on or make changes to this bug.