Bug 198724

Summary: [WinCairo] Remove build warning from RemoteInspector.
Product: WebKit Reporter: Basuke Suzuki <Basuke.Suzuki>
Component: PlatformAssignee: Basuke Suzuki <Basuke.Suzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: Basuke.Suzuki, commit-queue, darin, don.olmstead, ews-watchlist, hi, joepeck, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
PATCH
none
PATCH none

Description Basuke Suzuki 2019-06-10 11:16:41 PDT
In `inspector/remote/socket/RemoteInspectorConnectionClient.h` an interface was defined with empty implementation. It is method to be overwritten by sub classes so that parameter name is important. This empty method generates warning.

The subclasses of the class are just two and one has it's own implementation. It's good to define this as an abstract method and move default implementation to that sub class.
Comment 1 Basuke Suzuki 2019-06-10 11:32:32 PDT
Created attachment 371765 [details]
PATCH
Comment 2 Darin Adler 2019-06-10 13:46:55 PDT
Comment on attachment 371765 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=371765&action=review

> Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorConnectionClient.h:40
> +    virtual void didAccept(ConnectionID acceptedID, ConnectionID listenerID, Socket::Domain) = 0;

Alternative fix is:

    virtual void didAccept(ConnectionID /* acceptedID */, ConnectionID /* listenerID */, Socket::Domain) { }

There’s no need to switch to pure virtual just to get rid of the unused argument warning. Your suggested change is OK if we think it’s better for other reasons.
Comment 3 Basuke Suzuki 2019-06-10 14:44:53 PDT
(In reply to Darin Adler from comment #2)
> Alternative fix is:
> 
>     virtual void didAccept(ConnectionID /* acceptedID */, ConnectionID /*
> listenerID */, Socket::Domain) { }

That was also my first approach in downstream. The reason I changed my mind was I cannot be confident with that manner accepted by upstream. I will resend with commenting way. Thanks.
Comment 4 Basuke Suzuki 2019-06-10 14:49:38 PDT
Created attachment 371780 [details]
PATCH
Comment 5 Joseph Pecoraro 2019-06-10 15:50:12 PDT
Comment on attachment 371780 [details]
PATCH

r=me, let me know I you need me to cq+
Comment 6 Basuke Suzuki 2019-06-10 17:09:28 PDT
Comment on attachment 371780 [details]
PATCH

Thanks!
Comment 7 WebKit Commit Bot 2019-06-10 17:38:54 PDT
Comment on attachment 371780 [details]
PATCH

Clearing flags on attachment: 371780

Committed r246299: <https://trac.webkit.org/changeset/246299>
Comment 8 WebKit Commit Bot 2019-06-10 17:38:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-06-10 17:40:56 PDT
<rdar://problem/51605252>