Bug 193030 – [MSVC] error C2381: 'WebKit::NetworkHTTPSUpgradeChecker::~NetworkHTTPSUpgradeChecker': redefinition; '__declspec(noreturn)' or '[[noreturn]]' differs

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Fujii Hironori]

[MSVC] error C2381: 'WebKit::NetworkHTTPSUpgradeChecker::~NetworkHTTPSUpgradeChecker': redefinition; '__declspec(noreturn)' or '[[noreturn]]' differs

WinCairo MSVC Debug builds are failing to build.

https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Debug%20%28Build%29/builds/5276

> C:\WebKit-BuildWorker\wincairo-wkl-debug\build\Source\WebKit\NetworkProcess/NetworkHTTPSUpgradeChecker.cpp(82): error C2381: 'WebKit::NetworkHTTPSUpgradeChecker::~NetworkHTTPSUpgradeChecker': redefinition; '__declspec(noreturn)' or '[[noreturn]]' differs
> c:\webkit-buildworker\wincairo-wkl-debug\build\source\webkit\networkprocess\NetworkHTTPSUpgradeChecker.h(48): note: see declaration of 'WebKit::NetworkHTTPSUpgradeChecker::~NetworkHTTPSUpgradeChecker'

[Fujii Hironori]

Created attachment 358054 [details]
Patch

[Ross Kirsling]

Comment on attachment 358054 [details]
Patch

I added that #if and comment here as a WinCairo Debug build fix here, because it was very tricky to avoid breaking Cocoa Debug in the process:
https://bugs.webkit.org/show_bug.cgi?id=192482#c15

Looks like this issue is coming up again with the removal of ENABLE_HTTPS_UPGRADE in r239474 (bug 192937):
https://trac.webkit.org/changeset/239474/webkit

Your fix is certainly much more satisfying than mine was, and I think if Mac Debug EWS isn't complaining, we should be safe. Yay!

[Fujii Hironori]

I didn't check that. It looks pretty weird.

[Fujii Hironori]

Created attachment 358089 [details]
Try EWS

[Fujii Hironori]

Hmm, mac-debug-ews says "Passed tests". Weird.

[Chris Dumez]

Comment on attachment 358089 [details]
Try EWS

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

> Source/WebKit/UIProcess/WebPageProxy.h:1571
> +    NO_RETURN_DUE_TO_ASSERT void didFailToSuspendAfterProcessSwap();

This caused a lot of crashes last time around. Nothing has changed since AFAICT so this will very likely cause the same crashes in wk2 debug (no ews).

[Chris Dumez]

Comment on attachment 358089 [details]
Try EWS

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

> Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:48
> +    NO_RETURN_DUE_TO_ASSERT ~NetworkHTTPSUpgradeChecker();

This part of the change will likely be fine (iirc the crashes were in IPC related code and this is not called via IPC). However, I’d leave the WebPageProxy changes out as they did not work last time Ross tried them.

[Fujii Hironori]

Thank you for the comments. I'm going to land the patch withou IPC part changes.

[Fujii Hironori]

Created attachment 358093 [details]
Patch for landing

[Fujii Hironori]

Comment on attachment 358093 [details]
Patch for landing

Clearing flags on attachment: 358093

Committed r239555: <https://trac.webkit.org/changeset/239555>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46955123>