Bug 205951

Summary: Suppress "unused parameter" when including libWebRTC headers in LibWebRTCProviderCocoa.cpp
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebCore Misc.Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: DoNotImportToRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Wenson Hsieh 2020-01-08 12:42:48 PST 
Some unrelated changes to unified source ordering will cause this to become a build error in the future.
Comment 1 Wenson Hsieh 2020-01-08 12:47:30 PST 
Created attachment 387129 [details]
Patch
Comment 2 Wenson Hsieh 2020-01-08 13:16:45 PST 
Comment on attachment 387129 [details]
Patch

Thanks for the review!
Comment 3 WebKit Commit Bot 2020-01-08 14:10:24 PST 
Comment on attachment 387129 [details]
Patch

Clearing flags on attachment: 387129

Committed r254226: <https://trac.webkit.org/changeset/254226>
Comment 4 WebKit Commit Bot 2020-01-08 14:10:25 PST 
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2020-01-09 17:22:29 PST 
Why can’t we fix this in WebRTC and upstream the fix, instead?
Comment 6 youenn fablet 2020-01-10 00:49:20 PST 
(In reply to Darin Adler from comment #5)
> Why can’t we fix this in WebRTC and upstream the fix, instead?

We could try but I am not sure what the best way is.

One typical example is: virtual void OnDroppedFrame(DropReason reason) {}.

We could remove the parameter name but I am not sure this will align well with libwebrtc coding style. And adding an UNUSED_PARAM does not seem great.

We could also decide to only include the webrtc headers that we control (the ones in Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit) and move the ALLOW_UNUSED_PARAMETERS_XXX in those headers.
Comment 7 Darin Adler 2020-01-10 09:53:28 PST 
(In reply to youenn fablet from comment #6)
> One typical example is: virtual void OnDroppedFrame(DropReason reason) {}.
> 
> We could remove the parameter name but I am not sure this will align well
> with libwebrtc coding style.

I don’t know what libwebrtc coding style is, but I would propose this:

    virtual void OnDroppedFrame(DropReason /* reason */) {}

(In reply to youenn fablet from comment #6)
> We could also decide to only include the webrtc headers that we control (the
> ones in Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit) and move the
> ALLOW_UNUSED_PARAMETERS_XXX in those headers.

Yes, I think that would be a little better than distributing this responsibility across WebKit code.