Closed Bug 1550770 Opened 7 months ago Closed 7 months ago

Error instead of implicitly converting XPCOM interfaces to builtinclass

Categories

(Core :: XPCOM, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

XPCOM interfaces that contain notxpcom methods can't be implemented in JS. Right now, the XPIDL front end just marks it as builtinclass for you, but as Boris has pointed out, that can cause weird breakage if you just happen to mark a method notxpcom and don't realize the implication, and there's an implementation in JS.

I'll just convert it to produce an error, and convert all of the existing places that rely on the implicit builtinclass behavior. There might be some ripple effect because of the way implicit builtinclass inherits, but hopefully it won't be bad.

Type: defect → task
Priority: -- → P2
Depends on: 1550860
Depends on: 1550893

PIDL has the requirement that [scriptable] interfaces with [notxpcom]
methods or attributes are [builtinclass]. Currently, if you don't
explicitly mark something builtinclass when it should be, then the
XPIDL compiler will just silently treat it like builtinclass. This
means that you can cause the JS implementation of an XPCOM to start
failing without any warning by marking a method notxpcom.

This patch instead makes it an error. A prior patch fixed the existing
instances in the tree that relied on the implicit behavior.

I also added a test that we reject such classes missing builtinclass
at compile time, as well as classes that inherit from builtinclass
interfaces without themselves being builtinclass. I left behind a part
of the runtime test for this behavior, but now this test just ensures
that you can't implement a [builtinclass] interface in JS.

Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7005da6ab266
Error instead of implicitly converting XPCOM interfaces to builtinclass. r=nika
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.