The default bug view has changed. See this FAQ.

Need to ensure that the RHS of "implements" (or any other consequential interface) is not flagged as castable if used as an argument

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

4.69 KB, patch
khuey
: review+
Details | Diff | Splinter Review
7.96 KB, patch
peterv
: review+
Details | Diff | Splinter Review
11.02 KB, patch
peterv
: review+
Details | Diff | Splinter Review
8.43 KB, patch
peterv
: review+
Details | Diff | Splinter Review
4.70 KB, patch
peterv
: review+
Details | Diff | Splinter Review
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.
Created attachment 632873 [details] [diff] [review]
part 1.  Flag consequential interfaces in the WebIDL data model.
Attachment #632873 - Flags: review?(khuey)
Created attachment 632874 [details] [diff] [review]
part 2.  Add some tests for callback interfaces.
Attachment #632874 - Flags: review?(khuey)
Created attachment 632875 [details] [diff] [review]
part 3.  Clean up meaning of 'castable' and use thereof a bit.
Attachment #632875 - Flags: review?(khuey)
Created attachment 632876 [details] [diff] [review]
part 4.  Make sure consequential interface used as arguments are non-castable.
Attachment #632876 - Flags: review?(khuey)
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.
Attachment #632877 - Flags: review?(khuey)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #632873 - Flags: review?(khuey) → review+
Attachment #632874 - Flags: review?(khuey) → review?(peterv)
Attachment #632875 - Flags: review?(khuey) → review?(peterv)
Attachment #632876 - Flags: review?(khuey) → review?(peterv)
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".
Attachment #634533 - Flags: review?(peterv)
Attachment #632877 - Attachment is obsolete: true
Attachment #632877 - Flags: review?(khuey)
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.
Whiteboard: [need review] → [leave open][need review]
https://hg.mozilla.org/mozilla-central/rev/25a6e906d51b
Attachment #632874 - Flags: review?(peterv) → review+
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.
Attachment #632875 - Flags: review?(peterv) → review+
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.
Attachment #632876 - Flags: review?(peterv) → review+
Attachment #634533 - Flags: review?(peterv) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/cec6ea1283ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/8498c31fb92e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9f685ea9ed0
https://hg.mozilla.org/integration/mozilla-inbound/rev/298c8e888da5
Flags: in-testsuite+
Whiteboard: [leave open][need review]
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/cec6ea1283ad
https://hg.mozilla.org/mozilla-central/rev/8498c31fb92e
https://hg.mozilla.org/mozilla-central/rev/f9f685ea9ed0
https://hg.mozilla.org/mozilla-central/rev/298c8e888da5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.