Bug 191582 – [curl] warning: delete called on non-final 'WebCore::CurlDownload' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]

• Component: Platform
• Status: RESOLVED FIXED

[Fujii Hironori]

clang-cl reports following compilation warnings while building WinCairo port.

> [5002/6237] Building CXX object Source\WebCore\CMakeFiles\WebCore.dir\platform\network\curl\CurlDownload.cpp.obj
> In file included from ..\..\Source\WebCore\platform\network\curl\CurlDownload.cpp:29:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlDownload.h:32:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlRequest.h:32:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlRequestSchedulerClient.h:17:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlContext.h:30:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlSSLHandle.h:30:
> In file included from ..\..\WebKitLibraries\win\include\openssl/crypto.h:129:
> ..\..\WebKitLibraries\win\include\openssl/ossl_typ.h(86,9):  warning: Warning, overriding WinCrypt defines [-W#pragma-messages]
> #pragma message("Warning, overriding WinCrypt defines")
>         ^
> In file included from ..\..\Source\WebCore\platform\network\curl\CurlDownload.cpp:29:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlDownload.h:32:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlRequest.h:29:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlFormDataStream.h:30:
> In file included from ..\..\Source\WebCore\platform\network/FormData.h:22:
> In file included from ..\..\Source\WebCore\platform\network/BlobData.h:35:
> In file included from ..\..\Source\WebCore\platform\ThreadSafeDataBuffer.h:28:
> DerivedSources\ForwardingHeaders\wtf/ThreadSafeRefCounted.h(76,13):  warning: delete called on non-final 'WebCore::CurlDownload' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
>             delete static_cast<const T*>(this);
>             ^
> ..\..\Source\WebCore\platform\network\curl/CurlDownload.h(56,65):  note: in instantiation of member function 'WTF::ThreadSafeRefCounted<WebCore::CurlDownload, WTF::DestructionThread::Any>::deref' requested here
>     void deref() override { ThreadSafeRefCounted<CurlDownload>::deref(); }
>                                                                 ^
> In file included from ..\..\Source\WebCore\platform\network\curl\CurlDownload.cpp:29:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlDownload.h:32:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlRequest.h:29:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlFormDataStream.h:30:
> In file included from ..\..\Source\WebCore\platform\network/FormData.h:22:
> In file included from ..\..\Source\WebCore\platform\network/BlobData.h:35:
> In file included from ..\..\Source\WebCore\platform\ThreadSafeDataBuffer.h:28:
> DerivedSources\ForwardingHeaders\wtf/ThreadSafeRefCounted.h(80,13):  warning: delete called on non-final 'WebCore::CurlDownload' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
>             delete static_cast<const T*>(this);
>             ^
> 3 warnings generated.


> [5019/6237] Building CXX object Source\WebCore\CMakeFiles\WebCore.dir\platform\network\curl\NetworkStorageSessionCurl.cpp.obj
> In file included from ..\..\Source\WebCore\platform\network\curl\NetworkStorageSessionCurl.cpp:35:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlContext.h:30:
> In file included from ..\..\Source\WebCore\platform\network\curl/CurlSSLHandle.h:30:
> In file included from ..\..\WebKitLibraries\win\include\openssl/crypto.h:129:
> ..\..\WebKitLibraries\win\include\openssl/ossl_typ.h(86,9):  warning: Warning, overriding WinCrypt defines [-W#pragma-messages]
> #pragma message("Warning, overriding WinCrypt defines")
>         ^
> In file included from ..\..\Source\WebCore\platform\network\curl\NetworkStorageSessionCurl.cpp:26:
> In file included from ..\..\Source\WebCore\config.h:51:
> In file included from DerivedSources\ForwardingHeaders\wtf/FastMalloc.h:25:
> In file included from DerivedSources\ForwardingHeaders\wtf/StdLibExtras.h:30:
> C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\memory(2055,3):  warning: delete called on non-final 'WebCore::CookieJarCurlDatabase' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
>                 delete _Ptr;
>                 ^
> C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\memory(2267,4):  note: in instantiation of member function 'std::default_delete<WebCore::CookieJarCurlDatabase>::operator()' requested here
>                         this->get_deleter()(get());
>                         ^
> DerivedSources\ForwardingHeaders\wtf/UniqueRef.h(33,28):  note: in instantiation of member function 'std::unique_ptr<WebCore::CookieJarCurlDatabase, std::default_delete<WebCore::CookieJarCurlDatabase> >::~unique_ptr' requested here
> template<typename T> class UniqueRef;
>                            ^
> In file included from ..\..\Source\WebCore\platform\network\curl\NetworkStorageSessionCurl.cpp:26:
> In file included from ..\..\Source\WebCore\config.h:51:
> In file included from DerivedSources\ForwardingHeaders\wtf/FastMalloc.h:25:
> In file included from DerivedSources\ForwardingHeaders\wtf/StdLibExtras.h:30:
> C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\memory(2055,3):  warning: delete called on 'WebCore::CookieJarCurl' that is abstract but has non-virtual destructor [-Wdelete-non-virtual-dtor]
>                 delete _Ptr;
>                 ^
> C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\memory(2267,4):  note: in instantiation of member function 'std::default_delete<WebCore::CookieJarCurl>::operator()' requested here
>                         this->get_deleter()(get());
>                         ^
> DerivedSources\ForwardingHeaders\wtf/UniqueRef.h(42,7):  note: in instantiation of member function 'std::unique_ptr<WebCore::CookieJarCurl, std::default_delete<WebCore::CookieJarCurl> >::~unique_ptr' requested here
> class UniqueRef {
>       ^
> 3 warnings generated.

[Fujii Hironori]

Created attachment 354660 [details]
Patch

[Don Olmstead]

Comment on attachment 354660 [details]
Patch

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

I'm honestly wondering why there's a CookieJarCurl at all. There's no cpp file and I just see an implementation in CookieJarCurlDatabase. Maybe this should just be collapsed?

> Source/WebCore/platform/network/curl/CookieJarCurl.h:39
> +    virtual ~CookieJarCurl() { }

= default

[Basuke Suzuki]

(In reply to Don Olmstead from comment #2)
> Comment on attachment 354660 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354660&action=review
> 
> I'm honestly wondering why there's a CookieJarCurl at all. There's no cpp
> file and I just see an implementation in CookieJarCurlDatabase. Maybe this
> should just be collapsed?

This must be the left over of cookie backend abstraction. It had been a file based storage and when the introduction of sqlite based backend, the clients, such as NetworkStorageSession,  just used interface to communicate with backend.

I agree with removing this abstraction because we already took out the file based backend.

[Fujii Hironori]

(In reply to Basuke Suzuki from comment #3)
> I agree with removing this abstraction because we already took out the file
> based backend.

I did it.

   Bug 191620 – [curl] Unify CookieJarCurlDatabase and the abstract class CookieJarCurl

[Fujii Hironori]

Created attachment 354903 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 354903 [details]
Patch

Clearing flags on attachment: 354903

Committed r238263: <https://trac.webkit.org/changeset/238263>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46114410>