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] (Out June 25-July 6)
:
Mentors:
Depends on: 778150
Blocks: ParisBindings 749101
  Show dependency treegraph
 
Reported: 2012-05-21 12:18 PDT by Boris Zbarsky [:bz] (Out June 25-July 6)
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] (Out June 25-July 6)
peterv: review+
peterv: feedback+
Details | Diff | Review

Description Boris Zbarsky [:bz] (Out June 25-July 6) 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] (Out June 25-July 6) 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] (Out June 25-July 6) 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] (Out June 25-July 6) 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 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-07-13 16:30:39 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddfa6d5ad10a
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.