Bug 199586

Summary: Cleanup uses of NetworkProcess::m_sessionByConnection
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, commit-queue, ryanhaddad, sihui_liu, tsavell, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2019-07-08 14:00:56 PDT
Cleanup uses of NetworkProces::m_sessionByConnection.
Comment 1 Chris Dumez 2019-07-08 14:08:02 PDT
Created attachment 373661 [details]
Patch
Comment 2 Chris Dumez 2019-07-08 14:13:46 PDT
Created attachment 373663 [details]
Patch
Comment 3 Chris Dumez 2019-07-08 14:41:24 PDT
Created attachment 373666 [details]
Patch
Comment 4 Chris Dumez 2019-07-08 15:28:35 PDT
Comment on attachment 373666 [details]
Patch

Clearing flags on attachment: 373666

Committed r247230: <https://trac.webkit.org/changeset/247230>
Comment 5 Chris Dumez 2019-07-08 15:28:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Truitt Savell 2019-07-09 11:08:39 PDT
It looks like the changes in https://trac.webkit.org/changeset/247230/webkit
caused this test to crash on Debug WK2 for Mac and iOS: storage/domstorage/localstorage/private-browsing-affects-storage.html

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=storage%2Fdomstorage%2Flocalstorage%2Fprivate-browsing-affects-storage.html

Crash:
https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r247258%20(3475)/storage/domstorage/localstorage/private-browsing-affects-storage-crash-log.txt

I was able to reproduce this locally with command:

run-webkit-tests --root debug-247230 storage/domstorage/localstorage/private-browsing-affects-storage.html --iterations 20 -f --debug
Comment 7 Radar WebKit Bug Importer 2019-07-09 11:10:18 PDT
<rdar://problem/52842659>
Comment 8 Ryan Haddad 2019-07-09 13:57:10 PDT
(In reply to Truitt Savell from comment #6)
> It looks like the changes in https://trac.webkit.org/changeset/247230/webkit
> caused this test to crash on Debug WK2 for Mac and iOS:
> storage/domstorage/localstorage/private-browsing-affects-storage.html

ASSERTION FAILED: addResult.iterator->value == sessionID
/Volumes/Data/slave/mojave-debug/build/Source/WebKit/NetworkProcess/NetworkProcess.cpp(2670) : void WebKit::NetworkProcess::webPageWasAdded(IPC::Connection &, PAL::SessionID, WebCore::PageIdentifier, WebCore::PageIdentifier)

Chris, can you take a look at this or should we revert?
Comment 9 Truitt Savell 2019-07-09 14:01:54 PDT
Reverted r247230 for reason:

Caused storage/domstorage/localstorage/private-browsing-affects-storage.html to crash with an assertion.

Committed r247277: <https://trac.webkit.org/changeset/247277>
Comment 10 Chris Dumez 2019-07-09 14:06:09 PDT
(In reply to Ryan Haddad from comment #8)
> (In reply to Truitt Savell from comment #6)
> > It looks like the changes in https://trac.webkit.org/changeset/247230/webkit
> > caused this test to crash on Debug WK2 for Mac and iOS:
> > storage/domstorage/localstorage/private-browsing-affects-storage.html
> 
> ASSERTION FAILED: addResult.iterator->value == sessionID
> /Volumes/Data/slave/mojave-debug/build/Source/WebKit/NetworkProcess/
> NetworkProcess.cpp(2670) : void
> WebKit::NetworkProcess::webPageWasAdded(IPC::Connection &, PAL::SessionID,
> WebCore::PageIdentifier, WebCore::PageIdentifier)
> 
> Chris, can you take a look at this or should we revert?

Sihui, Alex, I believe my new assertion may have uncovered a bug in the existing code.
Comment 11 Chris Dumez 2019-07-09 14:49:02 PDT
Created attachment 373770 [details]
Patch
Comment 12 WebKit Commit Bot 2019-07-09 15:40:15 PDT
Comment on attachment 373770 [details]
Patch

Clearing flags on attachment: 373770

Committed r247283: <https://trac.webkit.org/changeset/247283>
Comment 13 WebKit Commit Bot 2019-07-09 15:40:17 PDT
All reviewed patches have been landed.  Closing bug.