793025 - Convert FileReaderSync to WebIDL bindings

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)]

Attachment: Patch

This also fixes a few codegen bugs I hit along the way.

Comment 1

[Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)]

Attachment: Patch

Comment 2

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 663198 [details] [diff] [review]
Patch

>+++ b/dom/bindings/Bindings.conf
>+    'headerFile': 'mozilla/dom/workers/bindings/FileReaderSync.h'

If we plan to keep putting stuff there for workers, we should just change the headerDefault for the worker case in Descriptor.__init__.  Followup bug on that is fine.

>+++ b/dom/webidl/FileReaderSync.webidl
>+  [Throws]
>+  DOMString readAsBinaryString(Blob blob);

Hmm.  This isn't in the spec...  Should it be?

Long-term, we may want to make this return ByteString (which we should add support for)...

>+++ b/dom/workers/FileReaderSync.cpp
>+FileReaderSync::Constructor(JSContext* aCx, JSObject* aGlobal,

Hrm.  So if a GC comes along after we call Wrap() but before caller is holding on to our JSObject, do we just end up with a dangling pointer?  This seems like a huge footgun.  Followup on making workers sane here?  :(

r=me

Comment 3

[Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4050caa0b9c5

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4050caa0b9c5

Comment 5

[Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)]

(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 663198 [details] [diff] [review]
> Patch
> 
> >+++ b/dom/bindings/Bindings.conf
> >+    'headerFile': 'mozilla/dom/workers/bindings/FileReaderSync.h'
> 
> If we plan to keep putting stuff there for workers, we should just change
> the headerDefault for the worker case in Descriptor.__init__.  Followup bug
> on that is fine.

Filed Bug 793528.

> >+++ b/dom/webidl/FileReaderSync.webidl
> >+  [Throws]
> >+  DOMString readAsBinaryString(Blob blob);
> 
> Hmm.  This isn't in the spec...  Should it be?
> 
> Long-term, we may want to make this return ByteString (which we should add
> support for)...

This method is deprecated.  I don't know what our removal plans are.

> >+++ b/dom/workers/FileReaderSync.cpp
> >+FileReaderSync::Constructor(JSContext* aCx, JSObject* aGlobal,
> 
> Hrm.  So if a GC comes along after we call Wrap() but before caller is
> holding on to our JSObject, do we just end up with a dangling pointer?  This
> seems like a huge footgun.  Followup on making workers sane here?  :(

Filed Bug 793529.