Bug 219160

Summary: [GTK][WPE] Migrate WebKitTestServer to libsoup 2.48 API
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: Tools / TestsAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cgarcia, cmarcelo, commit-queue, ews-watchlist, mcatanzaro, pgriffis, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=146109
Bug Depends on: 220922    
Bug Blocks: 219257    
Attachments:
Description Flags
Untested patch
none
Untested patch
none
Untested patch
cgarcia: review+
Patch
none
Patch mcatanzaro: review+

Frédéric Wang (:fredw)
Reported 2020-11-19 07:15:42 PST
Created attachment 414575 [details] Untested patch It seems our WebDriver code uses the new soup_server_listen APIs from 2.48, so the code added in bug 146165 is no longer correct, we should really require 2.48 or higher.
Attachments
Untested patch (1.93 KB, patch)
2020-11-19 07:15 PST, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Untested patch (1.93 KB, patch)
2020-11-19 07:16 PST, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Untested patch (1.93 KB, patch)
2020-11-19 07:17 PST, Frédéric Wang (:fredw) 		cgarcia: review+
DetailsFormatted DiffDiff
Patch (3.10 KB, patch)
2020-11-20 00:54 PST, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch (10.35 KB, patch)
2021-02-01 04:43 PST, Carlos Garcia Campos 		mcatanzaro: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Frédéric Wang (:fredw)
Comment 1 2020-11-19 07:16:25 PST
Created attachment 414576 [details] Untested patch
Frédéric Wang (:fredw)
Comment 2 2020-11-19 07:17:07 PST
Created attachment 414577 [details] Untested patch
Frédéric Wang (:fredw)
Comment 3 2020-11-19 09:52:16 PST
Comment on attachment 414577 [details] Untested patch Please take a look. Disclaimer: I haven't use glib for a long time, hopefully I haven't misused the API.
Frédéric Wang (:fredw)
Comment 4 2020-11-20 00:54:40 PST
Created attachment 414660 [details] Patch
EWS
Comment 5 2020-11-20 01:40:37 PST
Committed r270074: <https://trac.webkit.org/changeset/270074> All reviewed patches have been landed. Closing bug and clearing flags on attachment 414660 [details].
Radar WebKit Bug Importer
Comment 6 2020-11-20 01:41:16 PST
<rdar://problem/71620310>
WebKit Commit Bot
Comment 7 2021-01-25 07:06:46 PST
Re-opened since this is blocked by bug 220922
Carlos Garcia Campos
Comment 8 2021-02-01 04:43:28 PST
Created attachment 418858 [details] Patch
Michael Catanzaro
Comment 9 2021-02-01 06:54:17 PST
Comment on attachment 418858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418858&action=review > Tools/TestWebKitAPI/glib/WebKitGLib/WebKitTestServer.cpp:35 > + m_soupServer = adoptGRef(soup_server_new("server-header", "WebKitTestServer ", nullptr)); If options[ServerRunInThread] is set, I would go even further and create this on the WorkQueue thread using your trick with BinarySemaphore to dispatch a function and wait until it completes. Sharing m_soupServer across threads like the current code does might work in practice, but I doubt it's safe.
Carlos Garcia Campos
Comment 10 2021-02-01 07:09:28 PST
(In reply to Michael Catanzaro from comment #9) > Comment on attachment 418858 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=418858&action=review > > > Tools/TestWebKitAPI/glib/WebKitGLib/WebKitTestServer.cpp:35 > > + m_soupServer = adoptGRef(soup_server_new("server-header", "WebKitTestServer ", nullptr)); > > If options[ServerRunInThread] is set, I would go even further and create > this on the WorkQueue thread using your trick with BinarySemaphore to > dispatch a function and wait until it completes. Sharing m_soupServer across > threads like the current code does might work in practice, but I doubt it's > safe. Well, the creation and run happen before any test start, so it should be safe, I would say. We don't even need the run, and server callback could be a constructor parameter, but that would require all the tests and I don't think it's worth it.
Michael Catanzaro
Comment 11 2021-02-01 07:12:24 PST
I just like to be paranoid about sharing objects across threads. Anyway, it's probably OK. You have r+.
Carlos Garcia Campos
Comment 12 2021-02-02 00:50:35 PST
Committed r272195: <https://trac.webkit.org/changeset/272195>
Note You need to log in before you can comment on or make changes to this bug.