Summary: | REGRESSION (r231850): Cookie file cannot be read or written by network process | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sihui Liu <sihui_liu> | ||||||||||||
Component: | WebKit2 | Assignee: | Sihui Liu <sihui_liu> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, ggaren, realdawei, ryanhaddad, sihui_liu, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Sihui Liu
2018-06-19 09:45:16 PDT
Created attachment 343069 [details]
Patch
Comment on attachment 343069 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343069&action=review r=me This bug is super mysterious, and it should trigger in our API tests bug doesn't. I think it's worth your time to do a little more debugging to come up with a possibly more cohesive story of this bug -- maybe that path will teach us how to write a regression test for this. > Source/WebKit/ChangeLog:10 > + Default websiteDataStore may be added wrongly to network process before default session was > + set, as messages were asynchronous, so the cookie storage could be improperly set. Even though messages are async, they are guaranteed to arrive in order. So, I don't think it's possible for the AddWebsiteDataStore message to arrive before the Initialize message. (You can try this out if you're curious.) But, we do know by testing that moving the initialization of pending cookies to the initialization message fixes this bug. So I think that's a good change. Created attachment 343101 [details]
Patch for landing
Comment on attachment 343101 [details] Patch for landing Clearing flags on attachment: 343101 Committed r232989: <https://trac.webkit.org/changeset/232989> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #5) > Comment on attachment 343101 [details] > Patch for landing > > Clearing flags on attachment: 343101 > > Committed r232989: <https://trac.webkit.org/changeset/232989> Looks like this revision is causing API failures on Mac I reproduced locally and it failed, (previous revisions passed) Her are the commands I used to reproduce locally: ("testbuild-232989" is my local spade) run-api-tests --root testbuild-232989 --no-build --debug --verbose ServiceWorkers.ServiceWorkerAndCacheStorageSpecificDirectories and run-api-tests --root testbuild-232989 --no-build --debug --verbose ServiceWorkers.ServiceWorkerAndCacheStorageDefaultDirectories Sample output: https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/5150/steps/run-api-tests/logs/stdio Failed TestWebKitAPI.ServiceWorkers.ServiceWorkerAndCacheStorageDefaultDirectories Received data during response processing, queuing it. Received data during response processing, queuing it. /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1175 Value of: retrievedString.contains("/Caches/TestWebKitAPI/WebKit/CacheStorage") Actual: false Expected: true TestWebKitAPI.ServiceWorkers.ServiceWorkerAndCacheStorageSpecificDirectories Received data during response processing, queuing it. Received data during response processing, queuing it. /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1223 Value of: retrievedString.contains("\"path\": \"/var/tmp\"") Actual: false Expected: true Tracking test failure in https://bugs.webkit.org/show_bug.cgi?id=186897. This change was rolled out in https://trac.webkit.org/changeset/233044/webkit Created attachment 343291 [details]
Patch
*** Bug 186897 has been marked as a duplicate of this bug. *** Comment on attachment 343291 [details] Patch Clearing flags on attachment: 343291 Committed r233084: <https://trac.webkit.org/changeset/233084> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #12) > Comment on attachment 343291 [details] > Patch > > Clearing flags on attachment: 343291 > > Committed r233084: <https://trac.webkit.org/changeset/233084> Looks like we are getting an API test failure after this patch: https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/5195/steps/run-api-tests/logs/stdio Failed TestWebKitAPI.WebKit.WKHTTPCookieStoreWithoutProcessPool /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:471 Value of: message.UTF8String Actual: "PersistentCookieName=CookieValue" Expected: “PersistentCookieName=CookieValue; SessionCookieName=CookieValue" Missed a changed file in the patch which caused the API test failure. Running all API & Layout tests locally now to make sure the new patch doesn't cause any present tests to fail. (In reply to Sihui Liu from comment #15) > Missed a changed file in the patch which caused the API test failure. > Running all API & Layout tests locally now to make sure the new patch > doesn't cause any present tests to fail. cool :) Created attachment 343377 [details]
Patch for landing
Created attachment 343379 [details]
Patch for landing
Comment on attachment 343379 [details] Patch for landing Clearing flags on attachment: 343379 Committed r233108: <https://trac.webkit.org/changeset/233108> All reviewed patches have been landed. Closing bug. |