1333117 - partial interface definitions in files other than the file that declares the interface are a footgun

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

We have no documentation saying not to do this, nor do we have anything enforcing people don't do it.  But it doesn't work correctly, because of build dependency issues.

Specifically, if I do a build, then add "partial interface Navigator { void foo(); };" to Window.webidl, then do a dep build I will NOT get a "foo()" method on Navigator.  That's because NavigatorBinding.cpp had no previous dependency on Window.webidl, and we have no idea that there is now such a dependency and we need to regenerate NavigatorBinding.cpp.  So we don't regenerate it.

This is similar to the issue described in bug 1071490, where we removed the footgun in bug 1071615.  We should probably do something similar here...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Of course NavigatorProperty does just this.  I'll have to whitelist it; I also added notes about it needing a clobber on https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: We should ensure, at build-time, that partial interfaces are defined in the same file as the interface they extend, since our build system doesn't really support correct dep builds if they're placed in a different file

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8829521 [details] [diff] [review]
We should ensure, at build-time, that partial interfaces are defined in the same file as the interface they extend, since our build system doesn't really support correct dep builds if they're placed in a different file

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

Looks good. Worth throwing a test in to continue confirming this fails?

Comment 4

[Boris Zbarsky [:bzbarsky]]

We don't really have any infrastructure I know of for testing this, unfortunately.  That is, we don't have infra for testing that codegen failures (unlike parser failures) happen when they should.

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ah, ok, thanks.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0974b1960a9
We should ensure, at build-time, that partial interfaces are defined in the same file as the interface they extend, since our build system doesn't really support correct dep builds if they're placed in a different file.  r=qdot

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a0974b1960a9