Bug 176069

Created attachment 319424 [details] Add basic versions/stubs for Network Process files for wincairo webkit

Comment on attachment 319424 [details] Add basic versions/stubs for Network Process files for wincairo webkit View in context: https://bugs.webkit.org/attachment.cgi?id=319424&action=review > Source/WebKit/ChangeLog:8 > + * NetworkProcess/CustomProtocols/curl/LegacyCustomProtocolManagerCurl.cpp: Added. Let's not add an attempt to implement LegacyCustomProtocolManager. Implement API::URLSchemeTask instead.

(In reply to Alex Christensen from comment #2) > View in context: > https://bugs.webkit.org/attachment.cgi?id=319424&action=review > > > Source/WebKit/ChangeLog:8 > > + * NetworkProcess/CustomProtocols/curl/LegacyCustomProtocolManagerCurl.cpp: Added. > > Let's not add an attempt to implement LegacyCustomProtocolManager. > Implement API::URLSchemeTask instead. Thank you for your review. I'm working for this patch with Stephan Szabo. Why should not an attempt implementation be added to LegacyCustomProtocolManager? If it is not implemented, many LINK errors occur. For example, DerivedSources/WebKit2/LegacyCustomProtocolManagerMessageReceiver.cpp requires them. Those implementations are just to solve such LINK errors.

We should instead remove LegacyCustomProtocolManager.messages.in from the common WebKit2_MESSAGES_IN_FILES and add them in PlatformMac and PlatformGTK. We are moving in a direction of removing the LegacyCustomProtocolManager and we shouldn't make this even harder to do or tempt people to implement the wrong thing.

I just added https://bugs.webkit.org/show_bug.cgi?id=176230 for dealing with making it not required and will put up a new version of this without the LegacyCustomProtocolManager file.

Created attachment 319612 [details] Add basic versions/stubs for Network Process files for wincairo webkit (no LegacyCustomProtocolManager)

Comment on attachment 319612 [details] Add basic versions/stubs for Network Process files for wincairo webkit (no LegacyCustomProtocolManager) View in context: https://bugs.webkit.org/attachment.cgi?id=319612&action=review > Source/WebKit/NetworkProcess/win/SystemProxyWin.cpp:70 > + goto exitGracefully; > + > + wcstombs(buffer, tHost, bufferLen); > + result = true; > + > +exitGracefully: > + if (tRegBuffer) > + delete [] tRegBuffer; This is what std::unique_ptr<TCHAR[]> is for. Let's not add goto for this.

Created attachment 319655 [details] Add basic versions/stubs for Network Process files for wincairo webkit Cleanup for gotos, ensure wcstombs output ends up null terminated

Attachment 319655 [details] did not pass style-queue: ERROR: Source/WebKit/NetworkProcess/win/SystemProxyWin.cpp:47: Tab found; better to use spaces [whitespace/tab] [1] ERROR: Source/WebKit/NetworkProcess/win/SystemProxyWin.cpp:58: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 319655 [details] Add basic versions/stubs for Network Process files for wincairo webkit View in context: https://bugs.webkit.org/attachment.cgi?id=319655&action=review > Source/WebKit/NetworkProcess/Downloads/curl/DownloadCurl.cpp:29 > +#if !USE(NETWORK_SESSION) We should be planning to make a NETWORK_SESSION implementation for CURL soon. Mac, iOS, and GTK have already switched. The only reason we have this code in WebKit is because of El Capitan. Luckily a NetworkSession implementation is quite similar to a ResourceHandle implementation using asynchronous client callbacks.

Created attachment 319878 [details] Add basic versions/stubs for Network Process files for wincairo webkit (fixed styes errors) (In reply to Build Bot from comment #9) > ERROR: Source/WebKit/NetworkProcess/win/SystemProxyWin.cpp:47: Tab found; > better to use spaces [whitespace/tab] [1] > ERROR: Source/WebKit/NetworkProcess/win/SystemProxyWin.cpp:58: Tab found; > better to use spaces [whitespace/tab] [1] > Total errors found: 2 in 10 files Fixed these two style check errors.

Comment on attachment 319878 [details] Add basic versions/stubs for Network Process files for wincairo webkit (fixed styes errors) Clearing flags on attachment: 319878 Committed r221625: <http://trac.webkit.org/changeset/221625>

<rdar://problem/34694168>