Summary: | REGRESSION (r281516): imported/w3c/web-platform-tests/IndexedDB/serialize-sharedarraybuffer-throws.https.html is failing | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | ayumi_kojima | ||||||||||
Component: | Page Loading | Assignee: | Chris Dumez <cdumez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, beidson, cdumez, darin, ggaren, webkit-bot-watchers-bugzilla, 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=229565 https://bugs.webkit.org/show_bug.cgi?id=229610 |
||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 229559 | ||||||||||||
Attachments: |
|
Description
ayumi_kojima
2021-08-25 10:23:37 PDT
Updated test expectations https://trac.webkit.org/changeset/281561/webkit (In reply to ayumi_kojima from comment #2) > Updated test expectations https://trac.webkit.org/changeset/281561/webkit Thanks. The title says "flaky", but on most queues, it's a 100% failure. *** Bug 229565 has been marked as a duplicate of this bug. *** Created attachment 436548 [details]
WIP patch
Created attachment 436561 [details]
Patch
Created attachment 436564 [details]
Patch
Just FYI expectation is marked for imported/w3c/web-platform-tests/encoding/sharedarraybuffer.https.html here https://trac.webkit.org/changeset/281626/webkit The bug is already duped. Comment on attachment 436564 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436564&action=review > Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:639 > + // In case of a COOP process-swap, the old process gets a didFailProvisionalLoadWithErrorForFrame delegate call. We want to ignore Safari also uses didFailProvisionalLoadWithErrorForFrame. Do we want to prevent this call from WebKit in the case of a COOP process swap? (In reply to Alex Christensen from comment #11) > Comment on attachment 436564 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=436564&action=review > > > Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:639 > > + // In case of a COOP process-swap, the old process gets a didFailProvisionalLoadWithErrorForFrame delegate call. We want to ignore > > Safari also uses didFailProvisionalLoadWithErrorForFrame. Do we want to > prevent this call from WebKit in the case of a COOP process swap? I am not convinced we want to bypass this call at WebKit level. The old process starts a provisional load and this provisional load gets cancelled when we process-swap. I think calling didFailProvisionalLoadWithErrorForFrame() is correct behavior in the old process otherwise, the process would get a didStartProvisionalLoadForFrame call and then nothing else. Wouldn't it be bad too? I think the issue is really in WebKitTestRunner's injected bundle because the logic there thinks the test failed even though it merely proceeded in a new process. Comment on attachment 436564 [details]
Patch
Oh, looks like this is causing http/tests/contentextensions/block-everything-if-domain.html to fail. The test is expected to fail loading before committing any load so it triggers my new logic :/
Created attachment 436584 [details]
Patch
(In reply to Alex Christensen from comment #11) > Comment on attachment 436564 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=436564&action=review > > > Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:639 > > + // In case of a COOP process-swap, the old process gets a didFailProvisionalLoadWithErrorForFrame delegate call. We want to ignore > > Safari also uses didFailProvisionalLoadWithErrorForFrame. Do we want to > prevent this call from WebKit in the case of a COOP process swap? To be clear, I think we should call didFailProvisionalLoadWithErrorForFrame in the injected bundle in the old process (since it got a didStartProvisionalLoadForFrame and since the load gets cancelled in that process). However, we should definitely not call didFailProvisionalLoadWithErrorForFrame() at UIProcess level. We may have a bug there. I will follow-up to add a test and fix if we indeed have that bug. Comment on attachment 436584 [details]
Patch
EWS is happy
Committed r281699 (241049@main): <https://commits.webkit.org/241049@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 436584 [details]. |