Bug 183641

Summary:

Resource Load Statistics: Add clearing of storage access to WebResourceLoadStatisticsStore::clearInMemory()

Product:

WebKit

Reporter:

John Wilander <wilander>

Component:

WebKit Misc.

Assignee:

John Wilander <wilander>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cdumez, commit-queue, ews-watchlist, jlewis3, rniwa, ryanhaddad, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=183620

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews104 for mac-sierra-wk2	none
Archive of layout-test-results from ews123 for ios-simulator-wk2	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews107 for mac-sierra-wk2	none
Patch	none

John Wilander

Reported 2018-03-14 12:44:03 PDT

WebResourceLoadStatisticsStore::clearInMemory() should clear out all storage access entries in the network process. Right now, we have layout test failures because of one test providing access to another.

Attachments
Patch (19.33 KB, patch) 2018-03-14 12:53 PDT, John Wilander	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.58 MB, application/zip) 2018-03-14 14:06 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.30 MB, application/zip) 2018-03-14 14:26 PDT, EWS Watchlist	no flags	Details
Patch (22.57 KB, patch) 2018-03-14 16:45 PDT, John Wilander	no flags	Details Formatted Diff Diff
Patch (19.37 KB, patch) 2018-03-14 16:55 PDT, John Wilander	no flags	Details Formatted Diff Diff
Patch (22.60 KB, patch) 2018-03-14 17:30 PDT, John Wilander	no flags	Details Formatted Diff Diff
Patch (23.49 KB, patch) 2018-03-14 18:15 PDT, John Wilander	no flags	Details Formatted Diff Diff
Patch (23.96 KB, patch) 2018-03-14 18:58 PDT, John Wilander	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.66 MB, application/zip) 2018-03-14 20:08 PDT, EWS Watchlist	no flags	Details
Patch (19.47 KB, patch) 2018-03-14 20:41 PDT, John Wilander	no flags	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-03-14 12:44:34 PDT

<rdar://problem/38469497>

John Wilander

Comment 2 2018-03-14 12:53:09 PDT

Created attachment 335789 [details] Patch

Brent Fulgham

Comment 3 2018-03-14 13:02:41 PDT

Comment on attachment 335789 [details] Patch Looks good. r=me.

John Wilander

Comment 4 2018-03-14 13:04:48 PDT

(In reply to Brent Fulgham from comment #3) > Comment on attachment 335789 [details] > Patch > > Looks good. r=me. Thanks! I'll just wait for the EWS bubbles, then land.

EWS Watchlist

Comment 5 2018-03-14 14:06:07 PDT

Comment on attachment 335789 [details] Patch Attachment 335789 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6954405 New failing tests: http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store-one-hour.html

EWS Watchlist

Comment 6 2018-03-14 14:06:08 PDT

Created attachment 335795 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 7 2018-03-14 14:26:50 PDT

Comment on attachment 335789 [details] Patch Attachment 335789 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6954406 New failing tests: http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store-one-hour.html

EWS Watchlist

Comment 8 2018-03-14 14:26:52 PDT

Created attachment 335800 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6

John Wilander

Comment 9 2018-03-14 16:45:02 PDT

Created attachment 335809 [details] Patch

John Wilander

Comment 10 2018-03-14 16:45:29 PDT

Trying without the failing test case.

Chris Dumez

Comment 11 2018-03-14 16:49:16 PDT

Comment on attachment 335809 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335809&action=review > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:433 > + send(Messages::NetworkProcess::RemoveAllStorageAccess(sessionID), 0); To fix your crash, you may want to try: if (canSendMessage()) send(Messages::NetworkProcess::RemoveAllStorageAccess(sessionID), 0);

John Wilander

Comment 12 2018-03-14 16:54:36 PDT

(In reply to Chris Dumez from comment #11) > Comment on attachment 335809 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335809&action=review > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:433 > > + send(Messages::NetworkProcess::RemoveAllStorageAccess(sessionID), 0); > > To fix your crash, you may want to try: > if (canSendMessage()) > send(Messages::NetworkProcess::RemoveAllStorageAccess(sessionID), 0); Thanks, Chris! I will try this. If it works I should add that check to all the other message sends I have in this proxy.

John Wilander

Comment 13 2018-03-14 16:55:22 PDT

Created attachment 335811 [details] Patch

John Wilander

Comment 14 2018-03-14 17:30:51 PDT

Created attachment 335814 [details] Patch

John Wilander

Comment 15 2018-03-14 17:31:48 PDT

if (canSendMessage()) sadly did not fix the issue.

John Wilander

Comment 16 2018-03-14 18:15:20 PDT

It's marked as [ Pass ] in one more place. :(

John Wilander

Comment 17 2018-03-14 18:15:57 PDT

Created attachment 335817 [details] Patch

John Wilander

Comment 18 2018-03-14 18:58:30 PDT

Created attachment 335822 [details] Patch

EWS Watchlist

Comment 19 2018-03-14 20:08:44 PDT

Comment on attachment 335822 [details] Patch Attachment 335822 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6959540 New failing tests: http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store-one-hour.html

EWS Watchlist

Comment 20 2018-03-14 20:08:45 PDT

Created attachment 335825 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

Chris Dumez

Comment 21 2018-03-14 20:31:59 PDT

(In reply to John Wilander from comment #15) > if (canSendMessage()) sadly did not fix the issue. The crash is a bit different now: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebKit 0x000000010659abda WebKit::ChildProcessProxy::state() const + 4 (RefPtr.h:88) 1 com.apple.WebKit 0x0000000106636f33 WebKit::NetworkProcessProxy::removeAllStorageAccess(PAL::SessionID) + 21 (ChildProcessProxy.h:79) 2 com.apple.WebKit 0x000000010687e615 WebKit::WebsiteDataStore::removeAllStorageAccessHandler() + 139 (HashTable.h:181) 3 com.apple.JavaScriptCore 0x0000000105d7f78f WTF::RunLoop::performWork() + 447 (Function.h:56) 4 com.apple.JavaScriptCore 0x0000000105d7f942 WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39) 5 com.apple.CoreFoundation 0x00007fff86d133e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 6 com.apple.CoreFoundation 0x00007fff86cf465c __CFRunLoopDoSources0 + 556

Chris Dumez

Comment 22 2018-03-14 20:34:02 PDT

Comment on attachment 335822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335822&action=review > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1220 > + processPool->networkProcess()->removeAllStorageAccess(m_sessionID); I think chances are that networkProcess() returns null here. Try adding a null check for processPool->networkProcess().

John Wilander

Comment 23 2018-03-14 20:41:59 PDT

Created attachment 335828 [details] Patch

John Wilander

Comment 24 2018-03-14 20:43:16 PDT

(In reply to Chris Dumez from comment #22) > Comment on attachment 335822 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335822&action=review > > > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1220 > > + processPool->networkProcess()->removeAllStorageAccess(m_sessionID); > > I think chances are that networkProcess() returns null here. Try adding a > null check for processPool->networkProcess(). Thanks! New patch up.

WebKit Commit Bot

Comment 25 2018-03-14 22:05:41 PDT

Comment on attachment 335828 [details] Patch Clearing flags on attachment: 335828 Committed r229619: <https://trac.webkit.org/changeset/229619>

WebKit Commit Bot

Comment 26 2018-03-14 22:05:43 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.