932420 - Stop including windows.h in PeerConnectionImpl.h

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Boris Zbarsky [:bzbarsky]]

This screws over unified bindings, since we end up redefining random stuff.

The include path here is PeerConnectionImplBinding.cpp -> PeerConnectionImpl.h -> sigslot.h -> windows.h

We need to either break the include chain or sanitize this include by undefining all things that windows.h defines that would screw over other code.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: sigslot.h - #ifdef'ed out unused multi-threading to avoid windows.h

As discussed in #media, this #ifdef's out the multithreading parts of sigslot.h which we're not currently using in WebRTC, removing the #include "windows.h" as well.

Worked on my windows box. Try - https://tbpl.mozilla.org/?tree=Try&rev=5c84861b4e39

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 824264 [details] [diff] [review]
sigslot.h - #ifdef'ed out unused multi-threading to avoid windows.h

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

r+ with nits.  Asked question of bz in #developers, waiting for response, so hold off a bit on landing

::: media/mtransport/sigslot.h
@@ +94,5 @@
>  // On our copy of sigslot.h, we set single threading as default.
>  #define SIGSLOT_DEFAULT_MT_POLICY single_threaded
>  
> +// For now, this avoids windows.h in bindings (PeerConnectionImpl.h) on WIN32
> +#define SIGSLOT_LEAVE_OUT_MULTITHREADING 1

File a bug for the "real" solution (wrap win32 crit section and move to another file) and put the number here

@@ +104,5 @@
>  #	if !defined(WIN32_LEAN_AND_MEAN)
>  #		define WIN32_LEAN_AND_MEAN
>  #	endif
> +# ifndef SIGSLOT_LEAVE_OUT_MULTITHREADING
> +#	  include "windows.h"

Expand to include the LEAN_AND_MEAN

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: sigslot.h - #ifdef'ed out unused multi-threading to avoid windows.h (2) r=jesup

Updated with nits. Carrying forward r+ from jesup.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

(In reply to Randell Jesup [:jesup] from comment #2)
> r+ with nits.  Asked question of bz in #developers, waiting for response, so
> hold off a bit on landing

How long should we hold off? - Patch is ready to land fwiw.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/58ff751aed78

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/58ff751aed78