- Revision
- 190593
- Author
- [email protected]
- Date
- 2015-10-05 16:45:56 -0700 (Mon, 05 Oct 2015)
Log Message
REGRESSION(189668?): http/tests/notifications/events.html flakily asserts or times out
https://bugs.webkit.org/show_bug.cgi?id=149218
Reviewed by Alexey Proskuryakov.
Tools:
Because of r189668, WebKitTestRunner now tears down and recreates its WKNotificationManagerRef
when the TestOptions change. Previously, WebNotificationProvider only could handle a single
WKNotificationManagerRef. Because the ower of the WKNotificationManagerRef is reference counted,
and AppKit internally retains some objects which end up retaining the WKNotificationManagerRef,
the old WKNotificationManager may not be destroyed before the new one is created. Therefore,
WebNotificationProvider must be updated to appropriately handle multiple
WKNotificationManagerRefs in flight at the same time.
* WebKitTestRunner/WebNotificationProvider.cpp:
(WTR::WebNotificationProvider::~WebNotificationProvider):
(WTR::WebNotificationProvider::showWebNotification):
(WTR::WebNotificationProvider::closeWebNotification):
(WTR::WebNotificationProvider::addNotificationManager):
(WTR::WebNotificationProvider::removeNotificationManager):
(WTR::WebNotificationProvider::simulateWebNotificationClick):
(WTR::WebNotificationProvider::reset):
* WebKitTestRunner/WebNotificationProvider.h:
LayoutTests:
* TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (190592 => 190593)
--- trunk/LayoutTests/ChangeLog 2015-10-05 23:43:13 UTC (rev 190592)
+++ trunk/LayoutTests/ChangeLog 2015-10-05 23:45:56 UTC (rev 190593)
@@ -1,3 +1,12 @@
+2015-10-05 Myles C. Maxfield <[email protected]>
+
+ REGRESSION(189668?): http/tests/notifications/events.html flakily asserts or times out
+ https://bugs.webkit.org/show_bug.cgi?id=149218
+
+ Reviewed by Alexey Proskuryakov.
+
+ * TestExpectations:
+
2015-10-05 Dean Jackson <[email protected]>
Reference cycles during SVG dependency invalidation
Modified: trunk/LayoutTests/TestExpectations (190592 => 190593)
--- trunk/LayoutTests/TestExpectations 2015-10-05 23:43:13 UTC (rev 190592)
+++ trunk/LayoutTests/TestExpectations 2015-10-05 23:45:56 UTC (rev 190593)
@@ -676,5 +676,3 @@
# Marks as flaky (see also https://bugs.webkit.org/show_bug.cgi?id=132388)
http/tests/xmlhttprequest/timeout/xmlhttprequest-timeout-overrides.html [ Failure Pass ]
-
-webkit.org/b/149218 http/tests/notifications/events.html [ Pass Crash Timeout ]
Modified: trunk/Tools/ChangeLog (190592 => 190593)
--- trunk/Tools/ChangeLog 2015-10-05 23:43:13 UTC (rev 190592)
+++ trunk/Tools/ChangeLog 2015-10-05 23:45:56 UTC (rev 190593)
@@ -1,3 +1,28 @@
+2015-10-05 Myles C. Maxfield <[email protected]>
+
+ REGRESSION(189668?): http/tests/notifications/events.html flakily asserts or times out
+ https://bugs.webkit.org/show_bug.cgi?id=149218
+
+ Reviewed by Alexey Proskuryakov.
+
+ Because of r189668, WebKitTestRunner now tears down and recreates its WKNotificationManagerRef
+ when the TestOptions change. Previously, WebNotificationProvider only could handle a single
+ WKNotificationManagerRef. Because the ower of the WKNotificationManagerRef is reference counted,
+ and AppKit internally retains some objects which end up retaining the WKNotificationManagerRef,
+ the old WKNotificationManager may not be destroyed before the new one is created. Therefore,
+ WebNotificationProvider must be updated to appropriately handle multiple
+ WKNotificationManagerRefs in flight at the same time.
+
+ * WebKitTestRunner/WebNotificationProvider.cpp:
+ (WTR::WebNotificationProvider::~WebNotificationProvider):
+ (WTR::WebNotificationProvider::showWebNotification):
+ (WTR::WebNotificationProvider::closeWebNotification):
+ (WTR::WebNotificationProvider::addNotificationManager):
+ (WTR::WebNotificationProvider::removeNotificationManager):
+ (WTR::WebNotificationProvider::simulateWebNotificationClick):
+ (WTR::WebNotificationProvider::reset):
+ * WebKitTestRunner/WebNotificationProvider.h:
+
2015-10-05 Daniel Bates <[email protected]>
Disable Bitcode when building for iOS device
Modified: trunk/Tools/WebKitTestRunner/WebNotificationProvider.cpp (190592 => 190593)
--- trunk/Tools/WebKitTestRunner/WebNotificationProvider.cpp 2015-10-05 23:43:13 UTC (rev 190592)
+++ trunk/Tools/WebKitTestRunner/WebNotificationProvider.cpp 2015-10-05 23:45:56 UTC (rev 190593)
@@ -65,8 +65,8 @@
WebNotificationProvider::~WebNotificationProvider()
{
- if (m_currentNotificationManager)
- WKNotificationManagerSetProvider(m_currentNotificationManager.get(), nullptr);
+ for (auto& manager : m_ownedNotifications)
+ WKNotificationManagerSetProvider(manager.key.get(), nullptr);
}
WKNotificationProviderV0 WebNotificationProvider::provider()
@@ -84,38 +84,57 @@
return notificationProvider;
}
-void WebNotificationProvider::showWebNotification(WKPageRef, WKNotificationRef notification)
+void WebNotificationProvider::showWebNotification(WKPageRef page, WKNotificationRef notification)
{
- if (!m_currentNotificationManager)
- return;
-
+ auto context = WKPageGetContext(page);
+ auto notificationManager = WKContextGetNotificationManager(context);
uint64_t id = WKNotificationGetID(notification);
- ASSERT(!m_shownNotifications.contains(id));
- m_shownNotifications.add(id);
- WKNotificationManagerProviderDidShowNotification(m_currentNotificationManager.get(), WKNotificationGetID(notification));
+ ASSERT(m_ownedNotifications.contains(notificationManager));
+ auto addResult = m_ownedNotifications.find(notificationManager)->value.add(id);
+ ASSERT_UNUSED(addResult, addResult.isNewEntry);
+ auto addResult2 = m_owningManager.set(id, notificationManager);
+ ASSERT_UNUSED(addResult2, addResult2.isNewEntry);
+
+ WKNotificationManagerProviderDidShowNotification(notificationManager, id);
}
void WebNotificationProvider::closeWebNotification(WKNotificationRef notification)
{
- if (!m_currentNotificationManager)
- return;
+ uint64_t id = WKNotificationGetID(notification);
+ ASSERT(m_owningManager.contains(id));
+ auto notificationManager = m_owningManager.get(id);
- uint64_t id = WKNotificationGetID(notification);
+ ASSERT(m_ownedNotifications.contains(notificationManager));
+ bool success = m_ownedNotifications.find(notificationManager)->value.remove(id);
+ ASSERT_UNUSED(success, success);
+ m_owningManager.remove(id);
+
WKRetainPtr<WKUInt64Ref> wkID = WKUInt64Create(id);
WKRetainPtr<WKMutableArrayRef> array(AdoptWK, WKMutableArrayCreate());
WKArrayAppendItem(array.get(), wkID.get());
- m_shownNotifications.remove(id);
- WKNotificationManagerProviderDidCloseNotifications(m_currentNotificationManager.get(), array.get());
+ WKNotificationManagerProviderDidCloseNotifications(notificationManager, array.get());
}
void WebNotificationProvider::addNotificationManager(WKNotificationManagerRef manager)
{
- m_currentNotificationManager = manager;
+ m_ownedNotifications.add(manager, HashSet<uint64_t>());
}
void WebNotificationProvider::removeNotificationManager(WKNotificationManagerRef manager)
{
+ auto iterator = m_ownedNotifications.find(manager);
+ ASSERT(iterator != m_ownedNotifications.end());
+ auto toRemove = iterator->value;
+ WKRetainPtr<WKNotificationManagerRef> guard(manager);
+ m_ownedNotifications.remove(iterator);
+ WKRetainPtr<WKMutableArrayRef> array = adoptWK(WKMutableArrayCreate());
+ for (uint64_t notificationID : toRemove) {
+ bool success = m_owningManager.remove(notificationID);
+ ASSERT_UNUSED(success, success);
+ WKArrayAppendItem(array.get(), adoptWK(WKUInt64Create(notificationID)).get());
+ }
+ WKNotificationManagerProviderDidCloseNotifications(manager, array.get());
}
WKDictionaryRef WebNotificationProvider::notificationPermissions()
@@ -126,29 +145,23 @@
void WebNotificationProvider::simulateWebNotificationClick(uint64_t notificationID)
{
- if (!m_currentNotificationManager)
- return;
-
- ASSERT(m_shownNotifications.contains(notificationID));
- WKNotificationManagerProviderDidClickNotification(m_currentNotificationManager.get(), notificationID);
+ ASSERT(m_owningManager.contains(notificationID));
+ WKNotificationManagerProviderDidClickNotification(m_owningManager.get(notificationID), notificationID);
}
void WebNotificationProvider::reset()
{
- if (!m_currentNotificationManager) {
- m_shownNotifications.clear();
- return;
- }
+ for (auto& notificationPair : m_ownedNotifications) {
+ if (notificationPair.value.isEmpty())
+ continue;
+ WKRetainPtr<WKMutableArrayRef> array = adoptWK(WKMutableArrayCreate());
+ for (uint64_t notificationID : notificationPair.value)
+ WKArrayAppendItem(array.get(), adoptWK(WKUInt64Create(notificationID)).get());
- WKRetainPtr<WKMutableArrayRef> array(AdoptWK, WKMutableArrayCreate());
- HashSet<uint64_t>::const_iterator itEnd = m_shownNotifications.end();
- for (HashSet<uint64_t>::const_iterator it = m_shownNotifications.begin(); it != itEnd; ++it) {
- WKRetainPtr<WKUInt64Ref> wkID = WKUInt64Create(*it);
- WKArrayAppendItem(array.get(), wkID.get());
+ notificationPair.value.clear();
+ WKNotificationManagerProviderDidCloseNotifications(notificationPair.key.get(), array.get());
}
-
- m_shownNotifications.clear();
- WKNotificationManagerProviderDidCloseNotifications(m_currentNotificationManager.get(), array.get());
+ m_owningManager.clear();
}
} // namespace WTR
Modified: trunk/Tools/WebKitTestRunner/WebNotificationProvider.h (190592 => 190593)
--- trunk/Tools/WebKitTestRunner/WebNotificationProvider.h 2015-10-05 23:43:13 UTC (rev 190592)
+++ trunk/Tools/WebKitTestRunner/WebNotificationProvider.h 2015-10-05 23:45:56 UTC (rev 190593)
@@ -29,6 +29,7 @@
#include <WebKit/WKNotificationManager.h>
#include <WebKit/WKNotificationProvider.h>
#include <WebKit/WKRetainPtr.h>
+#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
namespace WTR {
@@ -49,8 +50,9 @@
void reset();
private:
- WKRetainPtr<WKNotificationManagerRef> m_currentNotificationManager;
- HashSet<uint64_t> m_shownNotifications;
+ // Inverses of each other.
+ HashMap<WKRetainPtr<WKNotificationManagerRef>, HashSet<uint64_t>> m_ownedNotifications;
+ HashMap<uint64_t, WKNotificationManagerRef> m_owningManager;
};
}
