Fix rooting issues in WebIDL dictionaries and ErrorResult

RESOLVED FIXED in mozilla23

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla23
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Let's get a start on reducing the dom/bindings noise.
(Assignee)

Comment 1

6 years ago
Created attachment 735957 [details] [diff] [review]
part 1.  Root WebIDL dictionary ParseJSON.   smaug
Attachment #735957 - Flags: review?(evilpies)
Attachment #735957 - Flags: review?(bugs)
(Assignee)

Updated

6 years ago
Assignee: nobody → bzbarsky
(Assignee)

Comment 2

6 years ago
Created attachment 735960 [details] [diff] [review]
part 2.  Root WebIDL dictionary ToObject.
Attachment #735960 - Flags: review?(evilpies)
Attachment #735960 - Flags: review?(bugs)
(Assignee)

Comment 3

6 years ago
Created attachment 735961 [details] [diff] [review]
part 3.  Root ErrorResult::ThrowJSException.
Attachment #735961 - Flags: review?(evilpies)
Attachment #735961 - Flags: review?(bugs)
(Assignee)

Updated

6 years ago
Whiteboard: [need review]
Comment on attachment 735957 [details] [diff] [review]
part 1.  Root WebIDL dictionary ParseJSON.   smaug

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

Uhh this pattern returning *cx is quite funny! Maybe<Rooted<Value>> is only used when and defined and everything lives on the stack, so this should be okay.

::: dom/bindings/Codegen.py
@@ +6818,5 @@
>                  ("  bool Init(const nsAString& aJSON)\n"
>                   "  {\n"
>                   "    mozilla::Maybe<JSAutoRequest> ar;\n"
>                   "    mozilla::Maybe<JSAutoCompartment> ac;\n"
> +                 "    Maybe< JS::Rooted<JS::Value> > json;\n"

Drop the mozilla:: for the others? Is that first space needed?
Attachment #735957 - Flags: review?(evilpies) → review+
Attachment #735960 - Flags: review?(evilpies) → review+
Comment on attachment 735961 [details] [diff] [review]
part 3.  Root ErrorResult::ThrowJSException.

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

Easy enough as well!
Attachment #735961 - Flags: review?(evilpies) → review+
(Assignee)

Comment 6

6 years ago
> Drop the mozilla:: for the others?

Done.

> Is that first space needed?

No, but I prefer the symmetry, and the last space _is_ needed.
Comment on attachment 735957 [details] [diff] [review]
part 1.  Root WebIDL dictionary ParseJSON.   smaug

Drop the space after <
Attachment #735957 - Flags: review?(bugs) → review+
Attachment #735960 - Flags: review?(bugs) → review+
Attachment #735961 - Flags: review?(bugs) → review+
You need to log in before you can comment on or make changes to this bug.