WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
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-
Details
Formatted Diff
Diff
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+
Details
Formatted Diff
Diff
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
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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
<
rdar://problem/34694168
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug