Last Comment Bug 764456 - Need to ensure that the RHS of "implements" (or any other consequential interface) is not flagged as castable if used as an argument
: Need to ensure that the RHS of "implements" (or any other consequential inter...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla17
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 742164
  Show dependency treegraph
 
Reported: 2012-06-13 09:55 PDT by Boris Zbarsky [:bz]
Modified: 2012-07-20 06:45 PDT (History)
3 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part 1. Flag consequential interfaces in the WebIDL data model. (4.69 KB, patch)
2012-06-13 14:12 PDT, Boris Zbarsky [:bz]
khuey: review+
Details | Diff | Splinter Review
part 2. Add some tests for callback interfaces. (7.96 KB, patch)
2012-06-13 14:12 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Splinter Review
part 3. Clean up meaning of 'castable' and use thereof a bit. (11.02 KB, patch)
2012-06-13 14:13 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Splinter Review
part 4. Make sure consequential interface used as arguments are non-castable. (8.43 KB, patch)
2012-06-13 14:13 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Splinter Review
part 5. Add a way to flag interfaces as not needing a prototype object so we won't do useless codegen for them. (2.78 KB, patch)
2012-06-13 14:15 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Splinter Review
part 5. Actually look at hasConcreteDescendant in hasInterfacePrototypeObject() so that we can avoid codegen for [NoInterfaceObject] interfaces hat are only implemented via "implements". (4.70 KB, patch)
2012-06-19 12:13 PDT, Boris Zbarsky [:bz]
peterv: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2012-06-13 09:55:12 PDT
In particular, we need a few changes here:

1)  Consequential interfaces need to be flagged as such.
2)  Codegen should assert if a consequential interface is used as an argument and
    is marked as castable.
3)  This-unwrapping should stop asserting that the interface being unwrapped to is
    castable, since it's possible for a concrete interface to be the RHS of
    implements and hence to require non-castability.
4)  Make a clear differentiation between our various interface types.  This will
    somewhat lay the groundwork for bug 742164.
Comment 1 Boris Zbarsky [:bz] 2012-06-13 14:12:45 PDT
Created attachment 632873 [details] [diff] [review]
part 1.  Flag consequential interfaces in the WebIDL data model.
Comment 2 Boris Zbarsky [:bz] 2012-06-13 14:12:56 PDT
Created attachment 632874 [details] [diff] [review]
part 2.  Add some tests for callback interfaces.
Comment 3 Boris Zbarsky [:bz] 2012-06-13 14:13:08 PDT
Created attachment 632875 [details] [diff] [review]
part 3.  Clean up meaning of 'castable' and use thereof a bit.
Comment 4 Boris Zbarsky [:bz] 2012-06-13 14:13:19 PDT
Created attachment 632876 [details] [diff] [review]
part 4.  Make sure consequential interface used as arguments are non-castable.
Comment 5 Boris Zbarsky [:bz] 2012-06-13 14:15:18 PDT
Created attachment 632877 [details] [diff] [review]
part 5.  Add a way to flag interfaces as not needing a prototype object so we won't do useless codegen for them.
Comment 6 Boris Zbarsky [:bz] 2012-06-19 12:13:19 PDT
Created attachment 634533 [details] [diff] [review]
part 5.  Actually look at hasConcreteDescendant in hasInterfacePrototypeObject() so that we can avoid codegen for [NoInterfaceObject] interfaces hat are only implemented via "implements".
Comment 7 Boris Zbarsky [:bz] 2012-06-22 13:25:37 PDT
Part 1 landed as https://hg.mozilla.org/integration/mozilla-inbound/rev/25a6e906d51b because other patches depended on it.  Bug remains open for parts 2-4.
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-06-23 05:45:27 PDT
https://hg.mozilla.org/mozilla-central/rev/25a6e906d51b
Comment 9 Peter Van der Beken [:peterv] 2012-07-12 08:00:23 PDT
Comment on attachment 632875 [details] [diff] [review]
part 3.  Clean up meaning of 'castable' and use thereof a bit.

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

Remove castable from addExternalIface.
Comment 10 Peter Van der Beken [:peterv] 2012-07-16 11:53:08 PDT
Comment on attachment 632876 [details] [diff] [review]
part 4.  Make sure consequential interface used as arguments are non-castable.

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

::: dom/bindings/Codegen.py
@@ +2806,5 @@
> +    def __init__(self, descriptor):
> +        self.castable = True
> +        self.workers = descriptor.workers
> +        self.nativeType = descriptor.nativeType
> +        self.name = descriptor.name

There are ways to have proxies in python I believe, but I guess that's a bit much for this one use case.

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