Bug 149218

Summary: REGRESSION(189668?): http/tests/notifications/events.html flakily asserts or times out
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Tools / TestsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bburg, commit-queue, mmaxfield, ryanhaddad, webkit-bug-importer, zalan
Priority: P1 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=149510
Bug Depends on: 149762, 149897    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alexey Proskuryakov 2015-09-16 09:31:15 PDT
http/tests/notifications/events.html flakily asserts or times out on Mac WK2:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&showExpectations=true&revision=189778&tests=http%2Ftests%2Fnotifications%2Fevents.html

This doesn't happen very often, but it definitely regressed within the last few days. r189668 seems suspicious, as it changed some notification permissions code in WKTR.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010f8c8ed7 WTFCrash + 39
1   WebKitTestRunner              	0x000000010ecf8474 WTR::WebNotificationProvider::simulateWebNotificationClick(unsigned long long) + 116 (WebNotificationProvider.cpp:132)
2   WebKitTestRunner              	0x000000010ed075ff WTR::TestController::simulateWebNotificationClick(unsigned long long) + 47 (TestController.cpp:1565)
3   WebKitTestRunner              	0x000000010ed193e0 WTR::TestInvocation::didReceiveMessageFromInjectedBundle(OpaqueWKString const*, void const*) + 2128 (TestInvocation.cpp:410)
4   WebKitTestRunner              	0x000000010ed06029 WTR::TestController::didReceiveMessageFromInjectedBundle(OpaqueWKString const*, void const*) + 2313 (TestController.cpp:1186)
5   WebKitTestRunner              	0x000000010ecfecfc WTR::TestController::didReceivePageMessageFromInjectedBundle(OpaqueWKPage const*, OpaqueWKString const*, void const*, void const*) + 44 (TestController.cpp:1072)
6   com.apple.WebKit              	0x0000000111a15380 WebKit::WebPageInjectedBundleClient::didReceiveMessageFromInjectedBundle(WebKit::WebPageProxy*, WTF::String const&, API::Object*) + 144 (WebPageInjectedBundleClient.cpp:42)
7   com.apple.WebKit              	0x0000000111a53a34 WebKit::WebPageProxy::handleMessage(IPC::Connection&, WTF::String const&, WebKit::UserData const&) + 228 (WebPageProxy.cpp:666)
Comment 1 Radar WebKit Bug Importer 2015-09-18 11:23:41 PDT
<rdar://problem/22760990>
Comment 2 zalan 2015-09-22 14:40:16 PDT
Committed r190135: <http://trac.webkit.org/changeset/190135>
Comment 3 zalan 2015-09-22 14:40:52 PDT
This is not resolved at all.
Comment 4 Alexey Proskuryakov 2015-09-22 22:23:54 PDT
> webkit.org/b/149218 http/tests/notifications/events.html [ Failure Pass ] 

This test actually never "fails" - it times out in release, and crashes in debug.
Comment 5 Myles C. Maxfield 2015-10-01 14:34:48 PDT
Looking at this. Seems likely caused by http://trac.webkit.org/changeset/189668
Comment 6 Myles C. Maxfield 2015-10-01 14:40:49 PDT
Doesn't look like I'm able to reproduce this on my local machine
Comment 7 Myles C. Maxfield 2015-10-01 17:56:28 PDT
Created attachment 262305 [details]
Patch
Comment 8 Anders Carlsson 2015-10-02 10:37:18 PDT
Comment on attachment 262305 [details]
Patch

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

> Tools/WebKitTestRunner/WebNotificationProvider.cpp:153
> +    auto iterator = m_owningManager.find(notificationID);
> +    ASSERT(iterator != m_owningManager.end());
> +    WKNotificationManagerProviderDidClickNotification(iterator->value.get(), notificationID);

No need to try to avoid an extra hash lookup that will only be triggered in debug code here, just use contains + get instead.
Comment 9 Myles C. Maxfield 2015-10-02 10:44:03 PDT
Comment on attachment 262305 [details]
Patch

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

> Tools/WebKitTestRunner/WebNotificationProvider.h:56
> +    // WKRetainPtr won't work with HashTable's deleted values (it tries to retain -1).
> +    // We can't teach it about HashTableDeletedValue because it lives in WTF.

This is not true. I should move the WKRetainPtr to m_ownedNotifications, and add HashTraits for it.
Comment 10 Myles C. Maxfield 2015-10-05 14:42:13 PDT
Created attachment 262462 [details]
Patch
Comment 11 Myles C. Maxfield 2015-10-05 16:46:27 PDT
Committed r190593: <http://trac.webkit.org/changeset/190593>
Comment 12 BJ Burg 2015-10-07 12:38:28 PDT
Reopening since this test still crashes occasionally:

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&revision=190660&tests=http%2Ftests%2Fnotifications%2Fevents.html

Here's a stack trace:

20:19:35.822 3776   ASSERTION FAILED: m_owningManager.contains(notificationID)
20:19:35.822 3776   /Volumes/Data/slave/mavericks-debug/build/Tools/WebKitTestRunner/WebNotificationProvider.cpp(148) : void WTR::WebNotificationProvider::simulateWebNotificationClick(uint64_t)
20:19:35.822 3776   1   0x10c3cedd0 WTFCrash
20:19:35.822 3776   2   0x10b7662ea WTR::WebNotificationProvider::simulateWebNotificationClick(unsigned long long)
20:19:35.822 3776   3   0x10b77ad8f WTR::TestController::simulateWebNotificationClick(unsigned long long)
20:19:35.822 3776   4   0x10b78c677 WTR::TestInvocation::didReceiveMessageFromInjectedBundle(OpaqueWKString const*, void const*)
20:19:35.822 3776   5   0x10b779765 WTR::TestController::didReceiveMessageFromInjectedBundle(OpaqueWKString const*, void const*)
20:19:35.822 3776   6   0x10b77223c WTR::TestController::didReceivePageMessageFromInjectedBundle(OpaqueWKPage const*, OpaqueWKString const*, void const*, void const*)
20:19:35.822 3776   7   0x10e5f24cb WebKit::WebPageInjectedBundleClient::didReceiveMessageFromInjectedBundle(WebKit::WebPageProxy*, WTF::String const&, API::Object*)
20:19:35.822 3776   8   0x10e6338f4 WebKit::WebPageProxy::handleMessage(IPC::Connection&, WTF::String const&, WebKit::UserData const&)
20:19:35.822 3776   9   0x10e6de03d void IPC::callMemberFunctionImpl<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&), std::__1::tuple<WTF::String, WebKit::UserData>, 0ul, 1ul>(WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&), IPC::Connection&, std::__1::tuple<WTF::String, WebKit::UserData>&&, std::index_sequence<0ul, 1ul>)
20:19:35.822 3776   10  0x10e6ddf70 void IPC::callMemberFunction<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&), std::__1::tuple<WTF::String, WebKit::UserData>, std::make_index_sequence<2ul> >(IPC::Connection&, std::__1::tuple<WTF::String, WebKit::UserData>&&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&))
20:19:35.822 3776   11  0x10e6d0fcf void IPC::handleMessage<Messages::WebPageProxy::HandleMessage, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&)>(IPC::Connection&, IPC::MessageDecoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&))
20:19:35.822 3776   12  0x10e6c371c WebKit::WebPageProxy::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
20:19:35.822 3776   13  0x10e6c39a7 non-virtual thunk to WebKit::WebPageProxy::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
20:19:35.823 3776   14  0x10dfe59ed IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::MessageDecoder&)
20:19:35.823 3776   15  0x10de88597 WebKit::ChildProcessProxy::dispatchMessage(IPC::Connection&, IPC::MessageDecoder&)
20:19:35.823 3776   16  0x10e7a6fca WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
20:19:35.823 3776   17  0x10e7a70c7 non-virtual thunk to WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
20:19:35.823 3776   18  0x10de96683 IPC::Connection::dispatchMessage(IPC::MessageDecoder&)
20:19:35.823 3776   19  0x10de8d76e IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)
20:19:35.823 3776   20  0x10de96c7f IPC::Connection::dispatchOneMessage()
20:19:35.823 3776   21  0x10de9836d IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10::operator()() const
20:19:35.823 3776   22  0x10de9833c std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10>, void ()>::operator()()
20:19:35.823 3776   23  0x10be8276a std::__1::function<void ()>::operator()() const
20:19:35.823 3776   24  0x10c419b54 WTF::RunLoop::performWork()
20:19:35.823 3776   25  0x10c41a2c4 WTF::RunLoop::performWork(void*)
20:19:35.823 3776   26  0x7fff915ae5b1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
20:19:35.823 3776   27  0x7fff9159fc62 __CFRunLoopDoSources0
20:19:35.823 3776   28  0x7fff9159f3ef __CFRunLoopRun
20:19:35.823 3776   29  0x7fff9159ee75 CFRunLoopRunSpecific
20:19:35.823 3776   30  0x7fff86cc516c -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
20:19:35.823 3776   31  0x10b798e8c WTR::TestController::platformRunUntil(bool&, double)
Comment 13 Ryan Haddad 2015-10-12 09:22:01 PDT
Created attachment 262891 [details]
Patch
Comment 14 BJ Burg 2015-10-12 09:35:30 PDT
Comment on attachment 262891 [details]
Patch

r=me
Comment 15 WebKit Commit Bot 2015-10-12 10:23:40 PDT
Comment on attachment 262891 [details]
Patch

Clearing flags on attachment: 262891

Committed r190858: <http://trac.webkit.org/changeset/190858>
Comment 16 WebKit Commit Bot 2015-10-12 10:23:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Ryan Haddad 2015-10-12 10:24:33 PDT
Reopening due to auto-close
Comment 18 Myles C. Maxfield 2016-07-01 17:10:29 PDT
https://bugs.webkit.org/show_bug.cgi?id=159375 fixes the flakiness.