From bc56563ac29548ab4d2ac78f36bd94f0fd1a3dba Mon Sep 17 00:00:00 2001 From: "jonlee@apple.com" Date: Wed, 22 Feb 2012 23:45:15 +0000 Subject: [PATCH] [WK2] Clearing notifications does not clean up internal state https://bugs.webkit.org/show_bug.cgi?id=78861 Reviewed by Anders Carlsson. * WebProcess/Notifications/WebNotificationManager.cpp: (WebKit::WebNotificationManager::clearNotifications): This function was missing removing the notification entries from the other maps. (WebKit::WebNotificationManager::didDestroyNotification): When the notification is destroyed, it should also be removed from the context map. (WebKit::WebNotificationManager::didCloseNotifications): Refactor to pull out the code that removes a provided notification from the context map. (WebKit::WebNotificationManager::removeNotificationFromContextMap): Find the notification in the map, and remove it. If the map is empty, get rid of the entry. * WebProcess/Notifications/WebNotificationManager.h: (WebNotificationManager): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108566 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebKit2/ChangeLog | 20 +++++++++++ .../Notifications/WebNotificationManager.cpp | 39 +++++++++++++++++----- .../Notifications/WebNotificationManager.h | 6 +++- 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog index 4db8124..15b2ccf 100644 --- a/Source/WebKit2/ChangeLog +++ b/Source/WebKit2/ChangeLog @@ -1,3 +1,23 @@ +2012-02-16 Jon Lee + + [WK2] Clearing notifications does not clean up internal state + https://bugs.webkit.org/show_bug.cgi?id=78861 + + + Reviewed by Anders Carlsson. + + * WebProcess/Notifications/WebNotificationManager.cpp: + (WebKit::WebNotificationManager::clearNotifications): This function was missing removing the notification + entries from the other maps. + (WebKit::WebNotificationManager::didDestroyNotification): When the notification is destroyed, it should + also be removed from the context map. + (WebKit::WebNotificationManager::didCloseNotifications): Refactor to pull out the code that removes + a provided notification from the context map. + (WebKit::WebNotificationManager::removeNotificationFromContextMap): Find the notification in the map, and + remove it. If the map is empty, get rid of the entry. + * WebProcess/Notifications/WebNotificationManager.h: + (WebNotificationManager): + 2012-02-22 Jon Lee Code cleanup in WebNotificationManager (79285) diff --git a/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp b/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp index 47ef930..90a7f40 100644 --- a/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp +++ b/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011 Apple Inc. All rights reserved. + * Copyright (C) 2011, 2012 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -145,7 +145,16 @@ void WebNotificationManager::clearNotifications(WebCore::ScriptExecutionContext* if (it == m_notificationContextMap.end()) return; - m_process->connection()->send(Messages::WebNotificationManagerProxy::ClearNotifications(it->second), page->pageID()); + Vector& notificationIDs = it->second; + m_process->connection()->send(Messages::WebNotificationManagerProxy::ClearNotifications(notificationIDs), page->pageID()); + size_t count = notificationIDs.size(); + for (size_t i = 0; i < count; ++i) { + RefPtr notification = m_notificationIDMap.take(notificationIDs[i]); + if (!notification) + continue; + m_notificationMap.remove(notification); + } + m_notificationContextMap.remove(it); #endif } @@ -157,7 +166,8 @@ void WebNotificationManager::didDestroyNotification(Notification* notification, if (!notificationID) return; - m_notificationIDMap.take(notificationID); + m_notificationIDMap.remove(notificationID); + removeNotificationFromContextMap(notificationID, notification); m_process->connection()->send(Messages::WebNotificationManagerProxy::DidDestroyNotification(notificationID), page->pageID()); #endif } @@ -199,19 +209,30 @@ void WebNotificationManager::didCloseNotifications(const Vector& notif if (!isNotificationIDValid(notificationID)) continue; - RefPtr notification = m_notificationIDMap.get(notificationID); + RefPtr notification = m_notificationIDMap.take(notificationID); if (!notification) continue; - NotificationContextMap::iterator it = m_notificationContextMap.find(notification->scriptExecutionContext()); - ASSERT(it != m_notificationContextMap.end()); - size_t index = it->second.find(notificationID); - ASSERT(index != notFound); - it->second.remove(index); + m_notificationMap.remove(notification); + removeNotificationFromContextMap(notificationID, notification.get()); notification->dispatchCloseEvent(); } #endif } +#if ENABLE(NOTIFICATIONS) +void WebNotificationManager::removeNotificationFromContextMap(uint64_t notificationID, Notification* notification) +{ + // This is a helper function for managing the hash maps. + NotificationContextMap::iterator it = m_notificationContextMap.find(notification->scriptExecutionContext()); + ASSERT(it != m_notificationContextMap.end()); + size_t index = it->second.find(notificationID); + ASSERT(index != notFound); + it->second.remove(index); + if (it->second.isEmpty()) + m_notificationContextMap.remove(it); +} +#endif + } // namespace WebKit diff --git a/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h b/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h index dc550e2..3b788be 100644 --- a/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h +++ b/Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011 Apple Inc. All rights reserved. + * Copyright (C) 2011, 2012 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -77,6 +77,10 @@ private: void didCloseNotifications(const Vector& notificationIDs); void didUpdateNotificationDecision(const String& originString, bool allowed); void didRemoveNotificationDecisions(const Vector& originStrings); + +#if ENABLE(NOTIFICATIONS) + void removeNotificationFromContextMap(uint64_t notificationID, WebCore::Notification*); +#endif WebProcess* m_process; -- 2.7.4