Closed Bug 949456 Opened 10 years ago Closed 10 years ago

Stop using "object" in MessageEventInit

Categories

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

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: bzbarsky, Assigned: baku)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Bug 767926 is long since fixed.  Sadly, we seem to have no folloup to fix this that depended on that bug...

Andrea, can you take this?
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Depends on: 949501
Depends on: 952073
Attached patch me.patch (obsolete) — Splinter Review
Attachment #8350183 - Flags: review?(bzbarsky)
Comment on attachment 8350183 [details] [diff] [review]
me.patch

>+  if (aParam.mSource.WasPassed() && !aParam.mSource.Value().IsNull()) {

If not passed and null are treated identically, just give it null as a default value in the IDL, please, and simplify this code.

> +      event->mPortSource = aParam.mSource.Value().Value().GetAsMessagePort().get();

Why do you need the .get()?

>+  init.mSource.Value().SetValue().SetAsMessagePort() = port;

That's wrong; you need to Construct() the Optional, no?  Yet another reason to have it default to null.
Attachment #8350183 - Flags: review?(bzbarsky) → review-
> > +      event->mPortSource = aParam.mSource.Value().Value().GetAsMessagePort().get();

 no known conversion for argument 1 from ‘mozilla::dom::OwningNonNull<mozilla::dom::MessagePortBase>’ to ‘const nsRefPtr<mozilla::dom::MessagePortBase>&’
Oh, I see.  That's moderately annoying, but I don't see a great way around it.   :(
Well, we could add an |operator T*| on (Owning)NonNull....
Attached patch me.patch (obsolete) — Splinter Review
Attachment #8350183 - Attachment is obsolete: true
Attachment #8350494 - Flags: review?(bzbarsky)
ah... you are on vacation. I can move this review request to somebody else.
Comment on attachment 8350494 [details] [diff] [review]
me.patch

>+  init.mSource.Value().SetAsMessagePort() = port;

That needs to be a SetValue(), I'd think.  What you have now will fatally assert.

r=me with that fixed.
Attachment #8350494 - Flags: review?(bzbarsky) → review+
Attached patch me.patchSplinter Review
https://tbpl.mozilla.org/?tree=Try&rev=6b4eada51f84

try before landing
Attachment #8350494 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/cd380f3e389e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: