Closed Bug 1204382 Opened 4 years ago Closed 4 years ago

--disable-webrtc builds are unable to compile MediaParent.cpp with errors about nsIInputStream being incomplete

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file, 1 obsolete file)

When building with --disable-webrtc, I get this build error:
{
dom/media/systemservices/MediaParent.cpp:169:40: error: no matching function for call to 'do_QueryInterface'
      nsCOMPtr<nsILineInputStream> i = do_QueryInterface(stream);
                                       ^~~~~~~~~~~~~~~~~
../../../dist/include/nsCOMPtr.h:188:1: note: candidate function not viable: no known conversion from 'nsCOMPtr<nsIInputStream>' to 'nsISupports *' for 1st argument
do_QueryInterface(nsISupports* aRawPtr)
^
}


This is a bit cryptic, but I get this hint from a bit further down in the build-error-spew:
{
../../../dist/include/nsCOMPtr.h:380:7: error: member access into incomplete type 'nsIInputStream'
      NSCAP_RELEASE(this, oldPtr);
      ^
[...]
../../../dist/include/nsIChannel.h:25:7: note: forward declaration of 'nsIInputStream'
class nsIInputStream; /* forward declaration */
}

Looks like we're just missing an #include to define what nsIInputStream is. I suspect in builds with WebRTC enabled, we get that #include indirectly from some other header. But not if you have --disable-webrtc.

(Note that this isn't the only issue I'm currently hitting with --disable-webrtc -- I also hit bug 1200477. But this one looks independent & easy to fix.)
Attached patch trivial fix: add #include (obsolete) — Splinter Review
Here's the (trivial) fix -- just add the #include for this type, so that we don't end up using it with only a forward-declaration to tell the compiler about it. (It's entirely appropriate for us to have this #include here, since this type is used in this file).

(Tangent: note that we *do* already have an include for nsI*Line*InputStream.h. I initially expected that that should be sufficient to provide nsIInputStream.h, because I thought it was a proper subclass -- but it is not. They're just two separate interfaces that both derive directly from nsISupports, and neither #includes the other.
http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsILineInputStream.idl
http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsIInputStream.idl
So we can't get away with just including one of them.
)
Attachment #8660509 - Flags: review?(jib)
Attached patch fix v2Splinter Review
Er, I'm reposting to tweak the #include order a bit.

Now this neighborhood of nsI* #includes are in alphabetical order, and also in the more logical order of "input, [special-type-of-input], output".
Attachment #8660509 - Attachment is obsolete: true
Attachment #8660509 - Flags: review?(jib)
Attachment #8660511 - Flags: review?(jib)
Comment on attachment 8660511 [details] [diff] [review]
fix v2

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

Thanks! Presumably it's on purpose that nsILineInputStream does not inherit from nsIInputStream (I see a lot of other interfaces inheriting from it, but not this one)?
Attachment #8660511 - Flags: review?(jib) → review+
Yeah, I'm not sure -- I haven't worked with either class before. Anyway, for now, an #include fixes the build error, and I'm not going to worry about the inheritance thing. :) Thanks for the review!
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/8b20e934953a
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Duplicate of this bug: 1202239
You need to log in before you can comment on or make changes to this bug.