Bug 207307

Summary: REGRESSION: [ macOS wk1 ] ASSERTION FAILED: _notifications.contains(notificationID) imported/w3c/web-platform-tests/notifications/constructor-basic.html is flaky crashing
Product: WebKit Reporter: Jacob Uphoff <jacob_uphoff>
Component: New BugsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, achristensen, beidson, cdumez, commit-queue, ggaren, tsavell, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=207237
Attachments:
Description Flags
Update Test Expectations
none
Test
none
Patch
none
Patch
none
Patch none

Description Jacob Uphoff 2020-02-05 16:20:30 PST
imported/w3c/web-platform-tests/notifications/constructor-basic.html

This test was first seen to have an assertion failure on 255440 

I  was not able to reproduce with iteration runs 5000x

History:

https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fnotifications%2Fconstructor-basic.html&limit=50000

Crash:

No crash log found for DumpRenderTree:29472.

stdout:

PASS Called the notification constructor with one argument. 


stderr:
ASSERTION FAILED: _notifications.contains(notificationID)
/Volumes/Data/slave/catalina-debug/build/Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm(76) : -[MockWebNotificationProvider cancelNotification:]
1   0x10bfc15d9 WTFCrash
2   0x10b15df6b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x10b1bd4ae -[MockWebNotificationProvider cancelNotification:]
4   0x112fe3a94 WebNotificationClient::cancel(WebCore::Notification*)
5   0x1266295dd WebCore::Notification::close()
6   0x126629698 WebCore::Notification::suspend(WebCore::ReasonForSuspension)
7   0x127321460 auto WebCore::ScriptExecutionContext::suspendActiveDOMObjects(WebCore::ReasonForSuspension)::$_6::operator()<WebCore::ActiveDOMObject>(WebCore::ActiveDOMObject&) const
8   0x1273213c1 WTF::Detail::CallableWrapper<WebCore::ScriptExecutionContext::suspendActiveDOMObjects(WebCore::ReasonForSuspension)::$_6, WebCore::ScriptExecutionContext::ShouldContinue, WebCore::ActiveDOMObject&>::call(WebCore::ActiveDOMObject&)
9   0x127302a97 WTF::Function<WebCore::ScriptExecutionContext::ShouldContinue (WebCore::ActiveDOMObject&)>::operator()(WebCore::ActiveDOMObject&) const
10  0x12730290a WebCore::ScriptExecutionContext::forEachActiveDOMObject(WTF::Function<WebCore::ScriptExecutionContext::ShouldContinue (WebCore::ActiveDOMObject&)> const&) const
11  0x127302b7e WebCore::ScriptExecutionContext::suspendActiveDOMObjects(WebCore::ReasonForSuspension)
12  0x127103fe4 WebCore::Document::suspendActiveDOMObjects(WebCore::ReasonForSuspension)
13  0x127101b69 WebCore::Document::suspendScheduledTasks(WebCore::ReasonForSuspension)
14  0x127116842 WebCore::Document::suspend(WebCore::ReasonForSuspension)
15  0x127529aa9 WebCore::CachedFrame::CachedFrame(WebCore::Frame&)
16  0x127529fed WebCore::CachedFrame::CachedFrame(WebCore::Frame&)
17  0x12753052d std::__1::__unique_if<WebCore::CachedFrame>::__unique_single std::__1::make_unique<WebCore::CachedFrame, WebCore::Frame&>(WebCore::Frame&)
18  0x127529e4f decltype(auto) WTF::makeUnique<WebCore::CachedFrame, WebCore::Frame&>(WebCore::Frame&)
19  0x12752a653 WebCore::CachedPage::CachedPage(WebCore::Page&)
20  0x12752a6bd WebCore::CachedPage::CachedPage(WebCore::Page&)
21  0x12752fc2d std::__1::__unique_if<WebCore::CachedPage>::__unique_single std::__1::make_unique<WebCore::CachedPage, WebCore::Page&>(WebCore::Page&)
22  0x127526c5f decltype(auto) WTF::makeUnique<WebCore::CachedPage, WebCore::Page&>(WebCore::Page&)
23  0x127526796 WebCore::BackForwardCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*)
24  0x127cabb8a WebCore::FrameLoader::commitProvisionalLoad()
25  0x127c3af6c WebCore::DocumentLoader::commitIfReady()
26  0x127c3b482 WebCore::DocumentLoader::finishedLoading()
27  0x127c45cee WebCore::DocumentLoader::maybeLoadEmpty()
28  0x127c45e73 WebCore::DocumentLoader::startLoadingMainResource()
29  0x127cd0fe7 WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, WebCore::NavigationPolicyDecision, WebCore::AllowNavigationToInvalidURL)::$_11::operator()()
30  0x127cd0b99 WTF::Detail::CallableWrapper<WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, WebCore::NavigationPolicyDecision, WebCore::AllowNavigationToInvalidURL)::$_11, void>::call()
31  0x1248fadca WTF::Function<void ()>::operator()() const
Comment 1 Radar WebKit Bug Importer 2020-02-05 16:21:20 PST
<rdar://problem/59206964>
Comment 2 Jacob Uphoff 2020-02-06 13:56:31 PST
Created attachment 389989 [details]
Update Test Expectations
Comment 3 Truitt Savell 2020-02-06 13:59:55 PST
Comment on attachment 389989 [details]
Update Test Expectations

Clearing flags on attachment: 389989

Committed r255977: <https://trac.webkit.org/changeset/255977>
Comment 4 Chris Dumez 2020-03-17 12:29:15 PDT
Created attachment 393775 [details]
Test

Test to reproduce the issue now that back/forward cache is disabled by default in DRT.
Comment 5 Chris Dumez 2020-03-17 14:06:38 PDT
Created attachment 393783 [details]
Patch
Comment 6 Alex Christensen 2020-03-17 14:09:25 PDT
Comment on attachment 393783 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=393783&action=review

> Tools/DumpRenderTree/mac/MockWebNotificationProvider.mm:147
> +        auto notification = _notifications.take(_notifications.begin()->key);

This unnecessarily rehashes a lot.  Why don't we just iterate then clear?
Comment 7 Chris Dumez 2020-03-17 14:24:00 PDT
Created attachment 393787 [details]
Patch
Comment 8 Aakash Jain 2020-03-17 16:44:09 PDT
Can you please verify if http/wpt/notifications/constructor-basic-bfcache.html failure on windows is due to your patch.

-PASS Called the notification constructor with one argument. 
+FAIL Called the notification constructor with one argument. Can't find variable: Notification
Comment 9 Chris Dumez 2020-03-17 17:10:09 PDT
Created attachment 393805 [details]
Patch
Comment 10 EWS 2020-03-17 17:12:52 PDT
Committed r258613: <https://trac.webkit.org/changeset/258613>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 393805 [details].
Comment 11 Ryan Haddad 2020-03-18 10:28:03 PDT
http/wpt/notifications/constructor-basic-bfcache.html is consistently failing on iOS bots:
https://build.webkit.org/results/Apple%20iOS%2013%20Simulator%20Release%20WK2%20(Tests)/r258630%20(3191)/results.html
Comment 12 Chris Dumez 2020-03-18 16:17:25 PDT
(In reply to Ryan Haddad from comment #11)
> http/wpt/notifications/constructor-basic-bfcache.html is consistently
> failing on iOS bots:
> https://build.webkit.org/results/
> Apple%20iOS%2013%20Simulator%20Release%20WK2%20(Tests)/r258630%20(3191)/
> results.html

Notifications are apparently meant to be skipped on iOS:
LayoutTests/platform/ios/TestExpectations:http/tests/notifications

We need to skip http/wpt/notifications too on this platform test.
Comment 13 Ryan Haddad 2020-03-18 16:22:34 PDT
(In reply to Chris Dumez from comment #12)
> Notifications are apparently meant to be skipped on iOS:
> LayoutTests/platform/ios/TestExpectations:http/tests/notifications
> 
> We need to skip http/wpt/notifications too on this platform test.
https://trac.webkit.org/changeset/258668/webkit