Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

imported/w3c/web-platform-tests/service-workers/service-worker/navigation-sets-cookie.https.html is flaky crashing #8577

Merged
merged 1 commit into from
Jan 12, 2023
Merged

imported/w3c/web-platform-tests/service-workers/service-worker/navigation-sets-cookie.https.html is flaky crashing #8577

merged 1 commit into from
Jan 12, 2023

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Jan 12, 2023 •
edited by webkit-early-warning-system

c41dfa7

imported/w3c/web-platform-tests/service-workers/service-worker/navigation-sets-cookie.https.html is flaky crashing
https://bugs.webkit.org/show_bug.cgi?id=250486
rdar://104138207

Reviewed by Youenn Fablet.

Have WebServiceWorkerFetchTaskClient keep the preloadResponse as a CrossThreadData
instead of a ResourceResponse. m_preloadResponse gets initialized on the service
worker thread but the WebServiceWorkerFetchTaskClient object always gets destroyed
on the main thread. A ResourceResponse contains AtomStrings and thus cannot be
destroyed on another thread than the one it got constructed on.

* Source/WebCore/workers/service/context/ServiceWorkerFetch.h:
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:
(WebCore::ServiceWorkerThreadProxy::navigationPreloadIsReady):
* Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:
(WebKit::WebServiceWorkerFetchTaskClient::setFetchEvent):
(WebKit::WebServiceWorkerFetchTaskClient::navigationPreloadIsReady):
* Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.h:

Canonical link: https://commits.webkit.org/258847@main

b3419ca

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ❌ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk   🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
  🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@cdumez cdumez self-assigned this Jan 12, 2023
@cdumez cdumez added the Service Workers Component for Service Workers bugs. label Jan 12, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash b3419ca)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ❌ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk   🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
  🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

@cdumez cdumez requested a review from youennf January 12, 2023 17:07
@cdumez cdumez mentioned this pull request Jan 12, 2023
Closed
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 12, 2023
@cdumez cdumez added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Jan 12, 2023
@cdumez
imported/w3c/web-platform-tests/service-workers/service-worker/naviga…
c41dfa7 
…tion-sets-cookie.https.html is flaky crashing

https://bugs.webkit.org/show_bug.cgi?id=250486
rdar://104138207

Reviewed by Youenn Fablet.

Have WebServiceWorkerFetchTaskClient keep the preloadResponse as a CrossThreadData
instead of a ResourceResponse. m_preloadResponse gets initialized on the service
worker thread but the WebServiceWorkerFetchTaskClient object always gets destroyed
on the main thread. A ResourceResponse contains AtomStrings and thus cannot be
destroyed on another thread than the one it got constructed on.

* Source/WebCore/workers/service/context/ServiceWorkerFetch.h:
* Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp:
(WebCore::ServiceWorkerThreadProxy::navigationPreloadIsReady):
* Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:
(WebKit::WebServiceWorkerFetchTaskClient::setFetchEvent):
(WebKit::WebServiceWorkerFetchTaskClient::navigationPreloadIsReady):
* Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.h:

Canonical link: https://commits.webkit.org/258847@main
@webkit-commit-queue
Copy link
Collaborator

Committed 258847@main (c41dfa7): https://commits.webkit.org/258847@main

Reviewed commits have been landed. Closing PR #8577 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit c41dfa7 into WebKit:main Jan 12, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youennf youennf youennf approved these changes

Assignees

@cdumez cdumez

Labels
Service Workers Component for Service Workers bugs.
Projects
None yet
Milestone
No milestone
5 participants
@cdumez @webkit-early-warning-system @webkit-commit-queue @youennf @webkit-ews-buildbot