Bug 186806 – REGRESSION (r231850): Cookie file cannot be read or written by network process

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Sihui Liu]

Safari signed out of Gmail/Twitter/Facebook/Reddit unexpectedly.

[Sihui Liu]

<rdar://problem/41113791>

[Sihui Liu]

Created attachment 343069 [details]
Patch

[Geoffrey Garen]

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.

[Sihui Liu]

Created attachment 343101 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 343101 [details]
Patch for landing

Clearing flags on attachment: 343101

Committed r232989: <https://trac.webkit.org/changeset/232989>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dawei Fenton (:realdawei)]

(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

[Sihui Liu]

Tracking test failure in https://bugs.webkit.org/show_bug.cgi?id=186897.

[Ryan Haddad]

This change was rolled out in https://trac.webkit.org/changeset/233044/webkit

[Sihui Liu]

Created attachment 343291 [details]
Patch

[Sihui Liu]

*** Bug 186897 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 343291 [details]
Patch

Clearing flags on attachment: 343291

Committed r233084: <https://trac.webkit.org/changeset/233084>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dawei Fenton (:realdawei)]

(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"

[Sihui Liu]

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.

[Dawei Fenton (:realdawei)]

(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 :)

[Sihui Liu]

Created attachment 343377 [details]
Patch for landing

[Sihui Liu]

Created attachment 343379 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 343379 [details]
Patch for landing

Clearing flags on attachment: 343379

Committed r233108: <https://trac.webkit.org/changeset/233108>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.