- Revision
- 202772
- Author
- mmaxfi...@apple.com
- Date
- 2016-07-01 19:53:11 -0700 (Fri, 01 Jul 2016)
Log Message
REGRESSION(r189668): Notification tests are flakey
https://bugs.webkit.org/show_bug.cgi?id=159375
<rdar://problem/22760990>
Reviewed by Alexey Proskuryakov.
Source/WebKit2:
Implement WKNotificationManagerGetLocalID(). For more information, see the entry in Tools/ChangeLog.
* UIProcess/API/C/WKNotificationManager.cpp:
(WKNotificationManagerGetLocalID):
* UIProcess/Notifications/WebNotificationManagerProxy.cpp:
(WebKit::WebNotificationManagerProxy::notificationLocalIDForTesting):
* UIProcess/Notifications/WebNotificationManagerProxy.h:
* WebKit2.xcodeproj/project.pbxproj:
Tools:
Notifications are objects which must exist in both the UI Process and the Web Process. Each process
identifies a notification object by a unique ID. When the Web Process sends a message regarding a
notification to the UI Process, the UI Process's WebNotificationManagerProxy holds a map from
(Page ID, Web Process notification ID) -> UI Process notification ID. This works as intended.
Our tests, however, include an additional method, simulateWebNotificationClick(), which is implemented
by WebKitTestRunner in the Web Process via the Injected Bundle. This method involves sending a message
to the UI process, to handle the simulated click. However, that RPC didn't perform the same local ->
global notification ID mapping, causing the wrong notification to be investigated.
The solution is for WebNotificationProvider, implemented in WebKitTestRunner in the UI Process, to
manually perform this same mapping. Luckily, this object already receives callbacks every time a
notification is created or destroyed. However, because this object is implemented outside WebKit,
it isn't privy to the internal Web Process ID used inside WebNotificationmanagerProxy. Therefore,
this patch adds a private testing function which returns this internal ID. Once given this intenal ID,
WebNotificationProvider can properly map between the different IDs.
* WebKitTestRunner/TestController.cpp:
(WTR::TestController::runTestingServerLoop):
(WTR::TestController::simulateWebNotificationClick):
* WebKitTestRunner/WebNotificationProvider.cpp:
(WTR::WebNotificationProvider::showWebNotification):
(WTR::removeGlobalIDFromIDMap):
(WTR::WebNotificationProvider::closeWebNotification):
(WTR::WebNotificationProvider::removeNotificationManager):
(WTR::WebNotificationProvider::simulateWebNotificationClick):
(WTR::WebNotificationProvider::reset):
* WebKitTestRunner/WebNotificationProvider.h:
LayoutTests:
* platform/mac/TestExpectations:
* platform/mac-wk2/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (202771 => 202772)
--- trunk/LayoutTests/ChangeLog 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/LayoutTests/ChangeLog 2016-07-02 02:53:11 UTC (rev 202772)
@@ -1,3 +1,14 @@
+2016-07-01 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ REGRESSION(r189668): Notification tests are flakey
+ https://bugs.webkit.org/show_bug.cgi?id=159375
+ <rdar://problem/22760990>
+
+ Reviewed by Alexey Proskuryakov.
+
+ * platform/mac/TestExpectations:
+ * platform/mac-wk2/TestExpectations:
+
2016-07-01 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r202766.
Modified: trunk/LayoutTests/platform/mac/TestExpectations (202771 => 202772)
--- trunk/LayoutTests/platform/mac/TestExpectations 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2016-07-02 02:53:11 UTC (rev 202772)
@@ -1263,9 +1263,6 @@
# Marking test as flaky in El Capitan
webkit.org/b/149819 [ Debug ElCapitan+ ] compositing/video/video-poster.html [ Pass Crash ]
-# Marking as flaky again, patch did not work
-webkit.org/b/149218 http/tests/notifications/events.html [ Pass Crash ]
-
# Flaky on Mac
webkit.org/b/148435 storage/domstorage/events/basic-body-attribute.html [ Pass Failure ]
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (202771 => 202772)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2016-07-02 02:53:11 UTC (rev 202772)
@@ -276,8 +276,6 @@
webkit.org/b/150241 webarchive/loading/object.html [ Pass Crash ]
-webkit.org/b/150736 http/tests/notifications/legacy/events.html [ Pass Crash ]
-
webkit.org/b/150942 animations/multiple-backgrounds.html [ Pass ImageOnlyFailure ]
webkit.org/b/151053 http/tests/security/cross-frame-access-put.html [ Pass Failure ]
Modified: trunk/Source/WebKit2/ChangeLog (202771 => 202772)
--- trunk/Source/WebKit2/ChangeLog 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Source/WebKit2/ChangeLog 2016-07-02 02:53:11 UTC (rev 202772)
@@ -1,3 +1,20 @@
+2016-07-01 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ REGRESSION(r189668): Notification tests are flakey
+ https://bugs.webkit.org/show_bug.cgi?id=159375
+ <rdar://problem/22760990>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Implement WKNotificationManagerGetLocalID(). For more information, see the entry in Tools/ChangeLog.
+
+ * UIProcess/API/C/WKNotificationManager.cpp:
+ (WKNotificationManagerGetLocalID):
+ * UIProcess/Notifications/WebNotificationManagerProxy.cpp:
+ (WebKit::WebNotificationManagerProxy::notificationLocalIDForTesting):
+ * UIProcess/Notifications/WebNotificationManagerProxy.h:
+ * WebKit2.xcodeproj/project.pbxproj:
+
2016-07-01 Jer Noble <jer.no...@apple.com>
REGRESSION(r201405): Fullscreen video no longer enters low-power mode
Modified: trunk/Source/WebKit2/UIProcess/API/C/WKNotificationManager.cpp (202771 => 202772)
--- trunk/Source/WebKit2/UIProcess/API/C/WKNotificationManager.cpp 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Source/WebKit2/UIProcess/API/C/WKNotificationManager.cpp 2016-07-02 02:53:11 UTC (rev 202772)
@@ -27,6 +27,7 @@
#include "WKNotificationManager.h"
#include "WKAPICast.h"
+#include "WebNotification.h"
#include "WebNotificationManagerProxy.h"
using namespace WebKit;
@@ -65,3 +66,8 @@
{
toImpl(managerRef)->providerDidRemoveNotificationPolicies(toImpl(origins));
}
+
+uint64_t WKNotificationManagerGetLocalIDForTesting(WKNotificationManagerRef manager, WKNotificationRef notification)
+{
+ return toImpl(manager)->notificationLocalIDForTesting(toImpl(notification));
+}
Modified: trunk/Source/WebKit2/UIProcess/API/C/WKNotificationManager.h (202771 => 202772)
--- trunk/Source/WebKit2/UIProcess/API/C/WKNotificationManager.h 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Source/WebKit2/UIProcess/API/C/WKNotificationManager.h 2016-07-02 02:53:11 UTC (rev 202772)
@@ -41,6 +41,7 @@
WK_EXPORT void WKNotificationManagerProviderDidCloseNotifications(WKNotificationManagerRef managerRef, WKArrayRef notificationIDs);
WK_EXPORT void WKNotificationManagerProviderDidUpdateNotificationPolicy(WKNotificationManagerRef managerRef, WKSecurityOriginRef origin, bool allowed);
WK_EXPORT void WKNotificationManagerProviderDidRemoveNotificationPolicies(WKNotificationManagerRef managerRef, WKArrayRef origins);
+WK_EXPORT uint64_t WKNotificationManagerGetLocalIDForTesting(WKNotificationManagerRef managerRef, WKNotificationRef notification);
#ifdef __cplusplus
}
Modified: trunk/Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.cpp (202771 => 202772)
--- trunk/Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.cpp 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.cpp 2016-07-02 02:53:11 UTC (rev 202772)
@@ -256,4 +256,16 @@
processPool()->sendToAllProcesses(Messages::WebNotificationManager::DidRemoveNotificationDecisions(originStrings));
}
+uint64_t WebNotificationManagerProxy::notificationLocalIDForTesting(WebNotification* notification)
+{
+ if (!notification)
+ return 0;
+
+ auto it = m_globalNotificationMap.find(notification->notificationID());
+ if (it == m_globalNotificationMap.end())
+ return 0;
+
+ return it->value.second;
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.h (202771 => 202772)
--- trunk/Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.h 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.h 2016-07-02 02:53:11 UTC (rev 202772)
@@ -67,6 +67,8 @@
void providerDidUpdateNotificationPolicy(const API::SecurityOrigin*, bool allowed);
void providerDidRemoveNotificationPolicies(API::Array* origins);
+ uint64_t notificationLocalIDForTesting(WebNotification*);
+
using API::Object::ref;
using API::Object::deref;
Modified: trunk/Tools/ChangeLog (202771 => 202772)
--- trunk/Tools/ChangeLog 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Tools/ChangeLog 2016-07-02 02:53:11 UTC (rev 202772)
@@ -1,3 +1,40 @@
+2016-07-01 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ REGRESSION(r189668): Notification tests are flakey
+ https://bugs.webkit.org/show_bug.cgi?id=159375
+ <rdar://problem/22760990>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Notifications are objects which must exist in both the UI Process and the Web Process. Each process
+ identifies a notification object by a unique ID. When the Web Process sends a message regarding a
+ notification to the UI Process, the UI Process's WebNotificationManagerProxy holds a map from
+ (Page ID, Web Process notification ID) -> UI Process notification ID. This works as intended.
+
+ Our tests, however, include an additional method, simulateWebNotificationClick(), which is implemented
+ by WebKitTestRunner in the Web Process via the Injected Bundle. This method involves sending a message
+ to the UI process, to handle the simulated click. However, that RPC didn't perform the same local ->
+ global notification ID mapping, causing the wrong notification to be investigated.
+
+ The solution is for WebNotificationProvider, implemented in WebKitTestRunner in the UI Process, to
+ manually perform this same mapping. Luckily, this object already receives callbacks every time a
+ notification is created or destroyed. However, because this object is implemented outside WebKit,
+ it isn't privy to the internal Web Process ID used inside WebNotificationmanagerProxy. Therefore,
+ this patch adds a private testing function which returns this internal ID. Once given this intenal ID,
+ WebNotificationProvider can properly map between the different IDs.
+
+ * WebKitTestRunner/TestController.cpp:
+ (WTR::TestController::runTestingServerLoop):
+ (WTR::TestController::simulateWebNotificationClick):
+ * WebKitTestRunner/WebNotificationProvider.cpp:
+ (WTR::WebNotificationProvider::showWebNotification):
+ (WTR::removeGlobalIDFromIDMap):
+ (WTR::WebNotificationProvider::closeWebNotification):
+ (WTR::WebNotificationProvider::removeNotificationManager):
+ (WTR::WebNotificationProvider::simulateWebNotificationClick):
+ (WTR::WebNotificationProvider::reset):
+ * WebKitTestRunner/WebNotificationProvider.h:
+
2016-07-01 Alexey Proskuryakov <a...@apple.com>
Simplify LayoutTestRelay
Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (202771 => 202772)
--- trunk/Tools/WebKitTestRunner/TestController.cpp 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp 2016-07-02 02:53:11 UTC (rev 202772)
@@ -1746,7 +1746,7 @@
void TestController::simulateWebNotificationClick(uint64_t notificationID)
{
- m_webNotificationProvider.simulateWebNotificationClick(notificationID);
+ m_webNotificationProvider.simulateWebNotificationClick(mainWebView()->page(), notificationID);
}
void TestController::setGeolocationPermission(bool enabled)
Modified: trunk/Tools/WebKitTestRunner/WebNotificationProvider.cpp (202771 => 202772)
--- trunk/Tools/WebKitTestRunner/WebNotificationProvider.cpp 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Tools/WebKitTestRunner/WebNotificationProvider.cpp 2016-07-02 02:53:11 UTC (rev 202772)
@@ -28,6 +28,7 @@
#include <WebKit/WKMutableArray.h>
#include <WebKit/WKNotification.h>
+#include <WebKit/WKNotificationManager.h>
#include <WebKit/WKNumber.h>
#include <WebKit/WKSecurityOriginRef.h>
#include <wtf/Assertions.h>
@@ -95,10 +96,23 @@
ASSERT_UNUSED(addResult, addResult.isNewEntry);
auto addResult2 = m_owningManager.set(id, notificationManager);
ASSERT_UNUSED(addResult2, addResult2.isNewEntry);
+ auto addResult3 = m_localToGlobalNotificationIDMap.add(std::make_pair(page, WKNotificationManagerGetLocalIDForTesting(notificationManager, notification)), id);
+ ASSERT_UNUSED(addResult3, addResult3.isNewEntry);
WKNotificationManagerProviderDidShowNotification(notificationManager, id);
}
+static void removeGlobalIDFromIDMap(HashMap<std::pair<WKPageRef, uint64_t>, uint64_t>& map, uint64_t id)
+{
+ for (auto iter = map.begin(); iter != map.end(); ++iter) {
+ if (iter->value == id) {
+ map.remove(iter);
+ return;
+ }
+ }
+ ASSERT_NOT_REACHED();
+}
+
void WebNotificationProvider::closeWebNotification(WKNotificationRef notification)
{
uint64_t id = WKNotificationGetID(notification);
@@ -110,6 +124,8 @@
ASSERT_UNUSED(success, success);
m_owningManager.remove(id);
+ removeGlobalIDFromIDMap(m_localToGlobalNotificationIDMap, id);
+
WKRetainPtr<WKUInt64Ref> wkID = WKUInt64Create(id);
WKRetainPtr<WKMutableArrayRef> array(AdoptWK, WKMutableArrayCreate());
WKArrayAppendItem(array.get(), wkID.get());
@@ -132,6 +148,7 @@
for (uint64_t notificationID : toRemove) {
bool success = m_owningManager.remove(notificationID);
ASSERT_UNUSED(success, success);
+ removeGlobalIDFromIDMap(m_localToGlobalNotificationIDMap, notificationID);
WKArrayAppendItem(array.get(), adoptWK(WKUInt64Create(notificationID)).get());
}
WKNotificationManagerProviderDidCloseNotifications(manager, array.get());
@@ -143,10 +160,12 @@
return WKMutableDictionaryCreate();
}
-void WebNotificationProvider::simulateWebNotificationClick(uint64_t notificationID)
+void WebNotificationProvider::simulateWebNotificationClick(WKPageRef page, uint64_t notificationID)
{
- ASSERT(m_owningManager.contains(notificationID));
- WKNotificationManagerProviderDidClickNotification(m_owningManager.get(notificationID), notificationID);
+ ASSERT(m_localToGlobalNotificationIDMap.contains(std::make_pair(page, notificationID)));
+ auto globalID = m_localToGlobalNotificationIDMap.get(std::make_pair(page, notificationID));
+ ASSERT(m_owningManager.contains(globalID));
+ WKNotificationManagerProviderDidClickNotification(m_owningManager.get(globalID), globalID);
}
void WebNotificationProvider::reset()
@@ -162,6 +181,7 @@
WKNotificationManagerProviderDidCloseNotifications(notificationPair.key.get(), array.get());
}
m_owningManager.clear();
+ m_localToGlobalNotificationIDMap.clear();
}
} // namespace WTR
Modified: trunk/Tools/WebKitTestRunner/WebNotificationProvider.h (202771 => 202772)
--- trunk/Tools/WebKitTestRunner/WebNotificationProvider.h 2016-07-02 02:32:39 UTC (rev 202771)
+++ trunk/Tools/WebKitTestRunner/WebNotificationProvider.h 2016-07-02 02:53:11 UTC (rev 202772)
@@ -46,7 +46,7 @@
void removeNotificationManager(WKNotificationManagerRef);
WKDictionaryRef notificationPermissions();
- void simulateWebNotificationClick(uint64_t notificationID);
+ void simulateWebNotificationClick(WKPageRef, uint64_t notificationID);
void reset();
private:
@@ -53,6 +53,8 @@
// Inverses of each other.
HashMap<WKRetainPtr<WKNotificationManagerRef>, HashSet<uint64_t>> m_ownedNotifications;
HashMap<uint64_t, WKNotificationManagerRef> m_owningManager;
+
+ HashMap<std::pair<WKPageRef, uint64_t>, uint64_t> m_localToGlobalNotificationIDMap;
};
}
