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

NEW
Assigned to

Status

()

defect
P3
normal
Rank:
25
4 years ago
2 years ago

People

(Reporter: gps, Assigned: nobody)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

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.
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/
Attachment #8731406 - Flags: review?(gsquelart)
Throwing it over the webrtc fence.
Assignee: nobody → gps
Component: Audio/Video → WebRTC
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.)
Attachment #8731406 - Flags: review?(gsquelart) → review?(rjesup)
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/
Attachment #8731406 - Attachment description: MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?gerald → MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup
Duplicate of this bug: 1256548
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 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?
Attachment #8731406 - Flags: review?(rjesup) → review+
Rank: 25
Priority: -- → P2
Attachment #8731406 - Attachment is obsolete: true
I'm landing the disabling of C4312 in bug 1124033. Repurposing this bug to track fixing the underlying warning.
No longer blocks: 1124033
Depends on: 1124033
Assignee: gps → nobody
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.
Flags: needinfo?(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.
Flags: needinfo?(pkerr)
Thank you. I'm assuming bug 1250356 tracks this work.
Depends on: 1250356
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
You need to log in before you can comment on or make changes to this bug.