1257317 - media\webrtc\trunk\webrtc/base/criticalsection.h(59): warning C4312: 'reinterpret_cast': conversion from 'DWORD' to 'HANDLE' of greater size from dom/media/systemservices

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P3)

Description

[Gregory Szorc [:gps]]

This warning is being unmasked in bug 1124033 and causes Visual Studio 2015 Update 1 builds to fail in automation due to warnings as errors.

Comment 1

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup

As part of unblocking building with VS2015u1 in automation, I'm mass
disabling compiler warnings that are turned into errors. This is not
the preferred mechanism to fix compilation warnings. So hopefully
this patch never lands because someone insists on fixing the underlying
problem instead. But if it does land, hopefully the workaround is
only temporary. That being said, this warning appears to occur in 3rd
party code. So our hands may be tied.

Review commit: https://reviewboard.mozilla.org/r/40557/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40557/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Throwing it over the webrtc fence.

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup

Randell, I think you should take over this review. Thanks.
(Can't see a way to change this in ReviewBoard.)

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40557/diff/1-2/

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40557/diff/2-3/

Comment 7

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

Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup

https://reviewboard.mozilla.org/r/40557/#review37233

I really shouldn't r+ changes to the ipc moz.build file...
even systemservices in dom/media is outside my direct purview as webrtc module owner, but I'm willing to r+ that.
Remove ipc, and r+ for the rest - but if you hope this won't land, file bugs for the warnings (and un-dup the criticalsection.h warning bug) so there's something to tell us what to fix.  Is it too much to hope for a "(bug blah)" in the build files where these are inserted?

Comment 8

[Gregory Szorc [:gps]]

I'm landing the disabling of C4312 in bug 1124033. Repurposing this bug to track fixing the underlying warning.

Comment 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Paul, I've got a fix for this, but I'm not sure whether it should be applied directly or through some patching system (to help with future updating from webrtc.org).

Changeset: https://hg.mozilla.org/try/rev/c0d888378c623c8f987a365af18157f8eb6709c8
From this try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c77bd3299bb22629aa103e4aa6172f4e84631c4

Note, this was also fixed on webrtc.org in Oct 2015:
https://chromium.googlesource.com/external/webrtc/+/a10492ff33152cef5a835761125219d3e4ee21d0%5E!/#F0
So if you are going to bring it here anytime soon, we can wait for that instead.

Comment 10

[Paul Kerr [:pkerr]]

Gerald, we are currently pulling in webrtc.org stable branch 49 into our codebase and should pick up the google fix in the process.

Comment 11

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you. I'm assuming bug 1250356 tracks this work.

Comment 12

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 13

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Fixed in Firefox 53 by bug 1250356.