Last Comment Bug 757164 - Move infallibility annotations into WebIDL
: Move infallibility annotations into WebIDL
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla16
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on: 778150
Blocks: ParisBindings 749101
  Show dependency treegraph
 
Reported: 2012-05-21 12:18 PDT by Boris Zbarsky [:bz]
Modified: 2012-07-27 08:12 PDT (History)
4 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Move infallibility annotations into webidl. this new setup, there are three new extended attributes: Infallible, (9.85 KB, patch)
2012-05-22 23:18 PDT, Boris Zbarsky [:bz]
peterv: review+
peterv: feedback+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2012-05-21 12:18:41 PDT
The current setup doesn't really scale very well, now that I've tried to convert CSSStyleDeclaration and the webgl context.
Comment 1 Boris Zbarsky [:bz] 2012-05-22 23:18:01 PDT
Created attachment 626337 [details] [diff] [review]
Move infallibility annotations into webidl.   this new setup, there are three new extended attributes: Infallible,
Comment 2 Boris Zbarsky [:bz] 2012-05-22 23:21:19 PDT
Comment on attachment 626337 [details] [diff] [review]
Move infallibility annotations into webidl.   this new setup, there are three new extended attributes: Infallible,

Peter, what do you think?  Both about the overall setup and the particular names I picked?

In some ways it would be nice to have a syntax that we could put after the attribute to avoid changing the line with the per-spec extended attributes, but I don't really want to hack up our parser if I don't have to....

I left the old mechanism in for now, but I can remove it if we think this works fine.

I would also be ok with nixing the worker/mainthread stuff and either requiring that things be flagged as infallible only if they're infallible on both workers and mainthread or that people who want to specify that sort of thing need to do so via the conf file...  But I think this setup is actually pretty simple.
Comment 3 Peter Van der Beken [:peterv] 2012-07-12 07:49:44 PDT
Comment on attachment 626337 [details] [diff] [review]
Move infallibility annotations into webidl.   this new setup, there are three new extended attributes: Infallible,

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

::: dom/bindings/Configuration.py
@@ +179,5 @@
>      def getExtendedAttributes(self, member, getter=False, setter=False):
>          name = member.identifier.name
>          if member.isMethod():
> +            attrs = self.extendedAttributes['all'].get(name, [])
> +            # Constructors seem to not have useful extended attr dicts?

What's this about? If that's why we have the |try| below I'd rather fix constructors. Aren't constructors IDLMethods?

@@ +188,5 @@
> +                if (infallible is not None and
> +                    (infallible is True or
> +                     ('Workers' in infallible and self.workers) or
> +                     ('MainThread' in infallible and not self.workers))):
> +                    attrs.append("infallible")

Do we want to throw for other values?

@@ +207,5 @@
> +        if (infallible is not None and
> +            (infallible is True or
> +             ('Workers' in infallible and self.workers) or
> +             ('MainThread' in infallible and not self.workers))):
> +            attrs.append("infallible")

Same here?
Comment 4 Boris Zbarsky [:bz] 2012-07-13 11:03:08 PDT
> If that's why we have the |try| below I'd rather fix constructors. Aren't constructors
> IDLMethods?

That's what it's about, yes.  Constructors are IDLMethods, but IDLInterfaceMember only sets up self._extendedAttrDict when addExtendedAttributes is called, and it's not called for constructors.

I'll just move that to IDLInterfaceMember.__init__.

> Do we want to throw for other values?

I can do that, yes.  Both places.
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-07-14 10:03:05 PDT
https://hg.mozilla.org/mozilla-central/rev/ddfa6d5ad10a

Note You need to log in before you can comment on or make changes to this bug.