RESOLVED FIXED 176069
[WinCairo] Add Network Process files for wincairo webkit
https://bugs.webkit.org/show_bug.cgi?id=176069
Summary [WinCairo] Add Network Process files for wincairo webkit
Stephan Szabo
Reported 2017-08-29 14:49:36 PDT
Basic versions of Network Process files for wincairo webkit2 for initial building.
Attachments
Add basic versions/stubs for Network Process files for wincairo webkit (25.43 KB, patch)
2017-08-30 16:58 PDT, Stephan Szabo
achristensen: review-
Add basic versions/stubs for Network Process files for wincairo webkit (no LegacyCustomProtocolManager) (22.05 KB, patch)
2017-09-01 10:42 PDT, Stephan Szabo
achristensen: review-
Add basic versions/stubs for Network Process files for wincairo webkit (21.97 KB, patch)
2017-09-01 15:37 PDT, Stephan Szabo
achristensen: review+
Add basic versions/stubs for Network Process files for wincairo webkit (fixed styes errors) (22.06 KB, patch)
2017-09-04 23:10 PDT, Yousuke Kimoto
no flags
Stephan Szabo
Comment 1 2017-08-30 16:58:41 PDT
Created attachment 319424 [details] Add basic versions/stubs for Network Process files for wincairo webkit
Alex Christensen
Comment 2 2017-08-30 17:35:47 PDT
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.
Yousuke Kimoto
Comment 3 2017-08-31 04:09:11 PDT
(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.
Alex Christensen
Comment 4 2017-08-31 19:01:31 PDT
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.
Stephan Szabo
Comment 5 2017-09-01 10:41:30 PDT
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.
Stephan Szabo
Comment 6 2017-09-01 10:42:19 PDT
Created attachment 319612 [details] Add basic versions/stubs for Network Process files for wincairo webkit (no LegacyCustomProtocolManager)
Alex Christensen
Comment 7 2017-09-01 10:44:45 PDT
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.
Stephan Szabo
Comment 8 2017-09-01 15:37:03 PDT
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
Build Bot
Comment 9 2017-09-01 15:39:29 PDT
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.
Alex Christensen
Comment 10 2017-09-01 15:44:33 PDT
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.
Yousuke Kimoto
Comment 11 2017-09-04 23:10:47 PDT
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.
WebKit Commit Bot
Comment 12 2017-09-05 11:08:25 PDT
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>
Radar WebKit Bug Importer
Comment 13 2017-09-27 12:51:56 PDT
Note You need to log in before you can comment on or make changes to this bug.