789691 - BlobPropertyBag doesn't work correctly in Workers.

Status: RESOLVED FIXED

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

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Type "new Worker('data:,throw new Blob([],{type:"aaa"}).type');" in Web Console.

Actual result:
An empty string will be thrown. ("uncaught exception: " displayed in Web Console.)

Expected result:
"aaa" should be thrown.

This is basically a legacy dictionary version of bug 788149.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Migrate BlobPropertyBag to WebIDL binding dictionary

Comment 2

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

Comment on attachment 659560 [details] [diff] [review]
Migrate BlobPropertyBag to WebIDL binding dictionary

I should not review any WebIDL usage.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 659560 [details] [diff] [review]
Migrate BlobPropertyBag to WebIDL binding dictionary

I'd prefer just adding "using namespace mozilla::dom" to nsDOMBlobBuilder.cpp, since eventually we'll want all that stuff to live in the mozilla::dom namespace anyway.

I don't understand why the test changes are needed.  Why are they needed?

As far as the actual Blob interface goes, I have two issues:

1)  Spec issue: Why does the contructor take a WebIDL array, not a sequence?  I'll raise a spec issue.

2)  We already have Blob defined as an external interface for now in XMLHttpRequest.webidl.  So the fact that this compiled and worked is a bit of an accident, and arguably a bug in the bindings codegen.  If we just need the Blob to force the dictionary to be generated, could we just use some other dummy interface instead?  Or go ahead and flip our Blob implementations to WebIDL and nix the external interface stuff for Blob?

r=me modulo those issues.

Comment 4

[Masatoshi Kimura [:emk]]

(In reply to Boris Zbarsky (:bz) from comment #3)
> I don't understand why the test changes are needed.  Why are they needed?
I just added a testcase for comment #0 to catch a future regression. The added test will fail without this change.

> 1)  Spec issue: Why does the contructor take a WebIDL array, not a sequence?
> I'll raise a spec issue.
Please, it doesn't matter at the moment because it is commented out right now.

> 2)  We already have Blob defined as an external interface for now in
> XMLHttpRequest.webidl.  So the fact that this compiled and worked is a bit
> of an accident, and arguably a bug in the bindings codegen.  If we just need
> the Blob to force the dictionary to be generated, could we just use some
> other dummy interface instead?  Or go ahead and flip our Blob
> implementations to WebIDL and nix the external interface stuff for Blob?
Blob.webidl only contains a dictionary definition because of the comment out. So the generated file will only contain a definition for the dictionary.
But I'm fine with renaming if you prefer.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Please, it doesn't matter at the moment because it is commented out right now.

Oh, I missed that.  OK, then that's fine.

> But I'm fine with renaming if you prefer.

No, given that the Blob interface is commented out this is fine.  Good to land!

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: patch for check in. r=bzbarsky

Added "using namespace mozilla::dom;" per the review comment.
I'll request checkin if the try is green.
https://tbpl.mozilla.org/?tree=Try&rev=2a6180996629

Comment 7

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52ad01f2b38e

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/52ad01f2b38e