1066703 - 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

Status: RESOLVED FIXED

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

Description

[Daniel Holbert [:dholbert]]

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

Comment 1

[Daniel Holbert [:dholbert]]

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?

Comment 2

[Jim Mathies [:jimm]]

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?

Comment 3

[Daniel Holbert [:dholbert]]

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.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Comment 5

[Daniel Holbert [:dholbert]]

(Note that we already have another usage of unused << in this file, so we're already getting the unused.h header somehow.)

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8488746 [details] [diff] [review]
fix v1

Thanks. I was just testing the same patch locally, builds fine w/without webrtc.

Comment 7

[Daniel Holbert [:dholbert]]

Thanks! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/d9b2292ab803

Comment 8

[Phil Ringnalda (:philor)]

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