Bug 198724 – [WinCairo] Remove build warning from RemoteInspector.

• Component: Platform
• Status: RESOLVED FIXED

[Basuke Suzuki]

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.

[Basuke Suzuki]

Created attachment 371765 [details]
PATCH

[Darin Adler]

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.

[Basuke Suzuki]

(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.

[Basuke Suzuki]

Created attachment 371780 [details]
PATCH

[Joseph Pecoraro]

Comment on attachment 371780 [details]
PATCH

r=me, let me know I you need me to cq+

[Basuke Suzuki]

Comment on attachment 371780 [details]
PATCH

Thanks!

[WebKit Commit Bot]

Comment on attachment 371780 [details]
PATCH

Clearing flags on attachment: 371780

Committed r246299: <https://trac.webkit.org/changeset/246299>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51605252>