Bug 176069 – [WinCairo] Add Network Process files for wincairo webkit

• Component: WebKit2
• Status: RESOLVED FIXED

[Stephan Szabo]

Basic versions of Network Process files for wincairo webkit2 for initial building.

[Stephan Szabo]

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

[Alex Christensen]

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]

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

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]

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]

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

[Alex Christensen]

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]

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]

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 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]

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 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]

<rdar://problem/34694168>