Closed Bug 919457 Opened 6 years ago Closed 6 years ago

Separate the concepts of "not available on the main thread" and "uses crazy worker stuff"

Categories

(Core :: DOM: Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
The attached patch separates workers: True and nativeOwnership: workers, and converts most of the workers: True objects away from inheriting from DOMBindingBase.
Attachment #808498 - Flags: review?(bzbarsky)
Assignee: nobody → khuey
Comment on attachment 808498 [details] [diff] [review]
Patch

>+++ b/dom/bindings/Codegen.py

>                    # Workers use raw pointers for new-object return
>                    # values or something
>-                   self.descriptor.workers)
>+                   self.descriptor.nativeOwnership == 'worker')

That should check the descriptor for self.returnType's interface, just like the 'owned' case above, no?

>+        if descriptorProvider.nativeOwnership == 'worker':

That's nonsense; a descriptor _provider_ doesn't have nativeOwnership...

On the other hand, callbacks on workers are actuall callbacks now.  Can you just nix this entire bit:

>+        if descriptorProvider.nativeOwnership == 'worker':
>             if type.nullable():
>                 return CGGeneric("JSObject*")
>             return CGGeneric("JSObject&")

please?  Looks like I forgot to do it.  :(

>+++ b/dom/workers/Location.h
>+#include "mozilla/dom/WorkerLocationBinding.h"

Put this in the .cpp, please.

>+    return WorkerLocationBinding_workers::Wrap(aCx, aScope, this);

And this bit.  It's virtual anyway, so not like it's getting inlined...

>+++ b/dom/workers/Navigator.h
>+#include "mozilla/dom/WorkerNavigatorBinding.h"

Likewise.

r=me with those fixed.
Attachment #808498 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/cd5ae0c57f7d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.