Closed Bug 1066703 Opened 5 years ago Closed 5 years ago

Build error: "MediaManager.h:225:17: error: private field 'mMediaThread' is not used [-Werror,-Wunused-private-field]" in --enable-warnings-as-errors --disable-webrtc build

Categories

(Core :: WebRTC: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

STR:
 1. Try to build mozilla-inbound (or m-c once bug 1060738 is merged over there) with this in your .mozconfig:

ac_add_options --enable-warnings-as-errors
ac_add_options --disable-webrtc

ACTUAL RESULTS:
Build error:
{
dom/media/MediaManager.h:225:17: error: private field 'mMediaThread' is not used [-Werror,-Wunused-private-field]
}

This is from the member-variable inside of GetUserMediaCallbackMediaStreamListener, which is only used in the #ifdef MOZ_WEBRTC chunk inside of GetUserMediaCallbackMediaStreamListener::AudioConfig().  So, it's indeed unused in disable-webrtc builds.

Link to its one usage:
https://hg.mozilla.org/integration/mozilla-inbound/annotate/75888bec4541/dom/media/MediaManager.cpp#l2388
Jim, looks like we need to better-scope the ifdeffing with respect to this variable/class.

A trivial solution would just be to add #ifdefs in the .h file, around this variable-declaration.  But maybe there's something higher-level (e.g. the GetUserMediaCallbackMediaStreamListener class itself) that should be #ifdef MOZ_WEBRTC?
Flags: needinfo?(jmathies)
Could we just do a little trick like 

if (mAudioSource && mMediaThread) {
#ifdef
#endif
}

to avoid this? Or is that still going to trigger the warning?
Flags: needinfo?(jmathies)
I believe that'd work, though if mMediaThread is guaranteed to be non-null at that point in the code, then that check would be a bit misleading & wasteful. ("wait, I thought this was initialized up there -- why are we null-checking it here? Could it have been cleared in the meantime?" etc.)

(on that note -- we probably should MOZ_ASSERT that the passed-in thread is non-null, in the GetUserMediaCallbackMediaStreamListener constructor.)

A better/cleaner hack would be to explicitly pass it to mozilla::unused, like:
 unused << mMediaThread;
in an #else clause after the #ifdef.
Attached patch fix v1Splinter Review
Attachment #8488746 - Flags: review?(jmathies)
(Note that we already have another usage of unused << in this file, so we're already getting the unused.h header somehow.)
Comment on attachment 8488746 [details] [diff] [review]
fix v1

Thanks. I was just testing the same patch locally, builds fine w/without webrtc.
Attachment #8488746 - Flags: review?(jmathies) → review+
Thanks! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/d9b2292ab803
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/d9b2292ab803
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.