Diff
Modified: trunk/Source/WebCore/ChangeLog (292450 => 292451)
--- trunk/Source/WebCore/ChangeLog 2022-04-06 06:06:15 UTC (rev 292450)
+++ trunk/Source/WebCore/ChangeLog 2022-04-06 07:05:25 UTC (rev 292451)
@@ -1,3 +1,20 @@
+2022-04-06 Youenn Fablet <you...@apple.com>
+
+ Remove Notification::m_relatedNotificationIdentifier
+ https://bugs.webkit.org/show_bug.cgi?id=238603
+
+ Reviewed by Brady Eidson.
+
+ m_relatedNotificationIdentifier was added as a temporary workaround for persistent notifications that are recreated in other contexts.
+ To directly support this case, Notification is no longer a Identified<UUID> and instead has a m_identifier.
+ When created from JS, Notification::m_identifier is generated.
+ When created from NotificationData, Notification::m_identifier is copied from NotificationData.
+
+ Covered by existing tests.
+
+ * Modules/notifications/Notification.cpp:
+ * Modules/notifications/Notification.h:
+
2022-04-05 Ada Chan <adac...@apple.com>
[WebXR] Add a new enum type to represent session features
Modified: trunk/Source/WebCore/Modules/notifications/Notification.cpp (292450 => 292451)
--- trunk/Source/WebCore/Modules/notifications/Notification.cpp 2022-04-06 06:06:15 UTC (rev 292450)
+++ trunk/Source/WebCore/Modules/notifications/Notification.cpp 2022-04-06 07:05:25 UTC (rev 292451)
@@ -58,7 +58,7 @@
if (context.isServiceWorkerGlobalScope())
return Exception { TypeError, "Notification cannot be directly created in a ServiceWorkerGlobalScope"_s };
- auto notification = adoptRef(*new Notification(context, WTFMove(title), WTFMove(options)));
+ auto notification = adoptRef(*new Notification(context, UUID::createVersion4(), WTFMove(title), WTFMove(options)));
notification->suspendIfNeeded();
notification->showSoon();
return notification;
@@ -66,7 +66,7 @@
Ref<Notification> Notification::createForServiceWorker(ScriptExecutionContext& context, String&& title, Options&& options, const URL& serviceWorkerRegistrationURL)
{
- auto notification = adoptRef(*new Notification(context, WTFMove(title), WTFMove(options)));
+ auto notification = adoptRef(*new Notification(context, UUID::createVersion4(), WTFMove(title), WTFMove(options)));
notification->m_serviceWorkerRegistrationURL = serviceWorkerRegistrationURL;
notification->suspendIfNeeded();
return notification;
@@ -75,15 +75,15 @@
Ref<Notification> Notification::create(ScriptExecutionContext& context, NotificationData&& data)
{
Options options { data.direction, WTFMove(data.language), WTFMove(data.body), WTFMove(data.tag), WTFMove(data.iconURL) };
- auto notification = adoptRef(*new Notification(context, WTFMove(data.title), WTFMove(options)));
+ auto notification = adoptRef(*new Notification(context, data.notificationID, WTFMove(data.title), WTFMove(options)));
notification->suspendIfNeeded();
- notification->m_relatedNotificationIdentifier = data.notificationID;
notification->m_serviceWorkerRegistrationURL = WTFMove(data.serviceWorkerRegistrationURL);
return notification;
}
-Notification::Notification(ScriptExecutionContext& context, String&& title, Options&& options)
+Notification::Notification(ScriptExecutionContext& context, UUID identifier, String&& title, Options&& options)
: ActiveDOMObject(&context)
+ , m_identifier(identifier)
, m_title(WTFMove(title).isolatedCopy())
, m_direction(options.dir)
, m_lang(WTFMove(options.lang).isolatedCopy())
@@ -106,30 +106,10 @@
}
}
-Notification::Notification(const Notification& other)
- : ActiveDOMObject(other.scriptExecutionContext())
- , m_title(other.m_title.isolatedCopy())
- , m_direction(other.m_direction)
- , m_lang(other.m_lang.isolatedCopy())
- , m_body(other.m_body.isolatedCopy())
- , m_tag(other.m_tag.isolatedCopy())
- , m_icon(other.m_icon.isolatedCopy())
- , m_state(other.m_state)
- , m_notificationSource(other.m_notificationSource)
- , m_contextIdentifier(other.m_contextIdentifier)
-{
- suspendIfNeeded();
-}
-
Notification::~Notification()
{
}
-Ref<Notification> Notification::copyForGetNotifications() const
-{
- return adoptRef(*new Notification(*this));
-}
-
void Notification::showSoon()
{
queueTaskKeepingObjectAlive(*this, TaskSource::UserInteraction, [this] {
@@ -222,6 +202,7 @@
void Notification::dispatchShowEvent()
{
ASSERT(isMainThread());
+ ASSERT(!isPersistent());
if (m_notificationSource != NotificationSource::Document)
return;
@@ -233,6 +214,8 @@
{
ASSERT(isMainThread());
ASSERT(m_notificationSource == NotificationSource::Document);
+ ASSERT(!isPersistent());
+
queueTaskKeepingObjectAlive(*this, TaskSource::UserInteraction, [this] {
WindowFocusAllowedIndicator windowFocusAllowed;
dispatchEvent(Event::create(eventNames().clickEvent, Event::CanBubble::No, Event::IsCancelable::No));
@@ -243,6 +226,8 @@
{
ASSERT(isMainThread());
ASSERT(m_notificationSource == NotificationSource::Document);
+ ASSERT(!isPersistent());
+
queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
finalize();
}
@@ -251,6 +236,7 @@
{
ASSERT(isMainThread());
ASSERT(m_notificationSource == NotificationSource::Document);
+ ASSERT(!isPersistent());
queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
}
Modified: trunk/Source/WebCore/Modules/notifications/Notification.h (292450 => 292451)
--- trunk/Source/WebCore/Modules/notifications/Notification.h 2022-04-06 06:06:15 UTC (rev 292450)
+++ trunk/Source/WebCore/Modules/notifications/Notification.h 2022-04-06 07:05:25 UTC (rev 292451)
@@ -38,8 +38,8 @@
#include "NotificationDirection.h"
#include "NotificationPermission.h"
#include "ScriptExecutionContextIdentifier.h"
-#include <wtf/Identified.h>
#include <wtf/URL.h>
+#include <wtf/UUID.h>
#include "WritingMode.h"
namespace WebCore {
@@ -51,7 +51,7 @@
struct NotificationData;
-class Notification final : public ThreadSafeRefCounted<Notification>, public ActiveDOMObject, public EventTargetWithInlineData, public UUIDIdentified<Notification> {
+class Notification final : public ThreadSafeRefCounted<Notification>, public ActiveDOMObject, public EventTargetWithInlineData {
WTF_MAKE_ISO_ALLOCATED_EXPORT(Notification, WEBCORE_EXPORT);
public:
using Permission = NotificationPermission;
@@ -98,8 +98,6 @@
WEBCORE_EXPORT NotificationData data() const;
- Ref<Notification> copyForGetNotifications() const;
-
using ThreadSafeRefCounted::ref;
using ThreadSafeRefCounted::deref;
@@ -106,11 +104,12 @@
void markAsShown();
void showSoon();
- std::optional<UUID> relatedNotificationIdentifier() const { return m_relatedNotificationIdentifier; }
+ UUID identifier() const { return m_identifier; }
+ bool isPersistent() const { return !m_serviceWorkerRegistrationURL.isNull(); }
+
private:
- Notification(ScriptExecutionContext&, String&& title, Options&&);
- Notification(const Notification&);
+ Notification(ScriptExecutionContext&, UUID, String&& title, Options&&);
NotificationClient* clientFromContext();
EventTargetInterface eventTargetInterface() const final { return NotificationEventTargetInterfaceType; }
@@ -126,6 +125,8 @@
void derefEventTarget() final { deref(); }
void eventListenersDidChange() final;
+ UUID m_identifier;
+
String m_title;
Direction m_direction;
String m_lang;
@@ -143,7 +144,6 @@
};
NotificationSource m_notificationSource;
ScriptExecutionContextIdentifier m_contextIdentifier;
- std::optional<UUID> m_relatedNotificationIdentifier;
URL m_serviceWorkerRegistrationURL;
};
Modified: trunk/Source/WebKit/ChangeLog (292450 => 292451)
--- trunk/Source/WebKit/ChangeLog 2022-04-06 06:06:15 UTC (rev 292450)
+++ trunk/Source/WebKit/ChangeLog 2022-04-06 07:05:25 UTC (rev 292451)
@@ -1,3 +1,16 @@
+2022-04-06 Youenn Fablet <you...@apple.com>
+
+ Remove Notification::m_relatedNotificationIdentifier
+ https://bugs.webkit.org/show_bug.cgi?id=238603
+
+ Reviewed by Brady Eidson.
+
+ Directly use identifier now that we correctly reuse them when needed.
+ We no longer store persistent notifications in the map since we do not use the map for click/close events.
+
+ * WebProcess/Notifications/WebNotificationManager.cpp:
+ * WebProcess/Notifications/WebNotificationManager.h:
+
2022-04-05 Ada Chan <adac...@apple.com>
[WebXR] Add a new enum type to represent session features
Modified: trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.cpp (292450 => 292451)
--- trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.cpp 2022-04-06 06:06:15 UTC (rev 292450)
+++ trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.cpp 2022-04-06 07:05:25 UTC (rev 292451)
@@ -130,7 +130,7 @@
#if ENABLE(BUILT_IN_NOTIFICATIONS)
if (RuntimeEnabledFeatures::sharedFeatures().builtInNotificationsEnabled())
- return m_process.ensureNetworkProcessConnection().connection().send(WTFMove(message), notification.data().sourceSession.toUInt64());
+ return m_process.ensureNetworkProcessConnection().connection().send(WTFMove(message), WebProcess::singleton().sessionID().toUInt64());
#endif
std::optional<WebCore::PageIdentifier> pageIdentifier;
@@ -160,7 +160,10 @@
if (!sendNotificationMessage(Messages::NotificationManagerMessageHandler::ShowNotification(notification.data()), notification, page))
return false;
- m_notificationIDMap.add(notification.identifier(), ¬ification);
+ if (!notification.isPersistent()) {
+ ASSERT(!m_nonPersistentNotifications.contains(notification.identifier()));
+ m_nonPersistentNotifications.add(notification.identifier(), notification);
+ }
return true;
#else
UNUSED_PARAM(notification);
@@ -175,14 +178,7 @@
#if ENABLE(NOTIFICATIONS)
auto identifier = notification.identifier();
- auto mappedNotification = m_notificationIDMap.get(identifier);
- if (!mappedNotification) {
- auto relatedIdentifier = notification.relatedNotificationIdentifier();
- if (!relatedIdentifier)
- return;
- identifier = *relatedIdentifier;
- } else
- ASSERT(mappedNotification == ¬ification);
+ ASSERT(notification.isPersistent() || !m_nonPersistentNotifications.contains(identifier) || m_nonPersistentNotifications.get(identifier) == ¬ification);
if (!sendNotificationMessage(Messages::NotificationManagerMessageHandler::CancelNotification(identifier), notification, page))
return;
@@ -200,14 +196,8 @@
Ref protectedNotification { notification };
auto identifier = notification.identifier();
- auto takenNotification = m_notificationIDMap.take(identifier);
- if (!takenNotification) {
- auto relatedIdentifier = notification.relatedNotificationIdentifier();
- if (!relatedIdentifier)
- return;
- identifier = *relatedIdentifier;
- } else
- ASSERT(takenNotification == ¬ification);
+ RefPtr takenNotification = !notification.isPersistent() ? m_nonPersistentNotifications.take(identifier) : nullptr;
+ ASSERT(!takenNotification || takenNotification == ¬ification);
sendNotificationMessage(Messages::NotificationManagerMessageHandler::DidDestroyNotification(identifier), notification, page);
#else
@@ -223,7 +213,7 @@
LOG(Notifications, "WebProcess %i DID SHOW notification %s", getpid(), notificationID.toString().utf8().data());
#if ENABLE(NOTIFICATIONS)
- RefPtr<Notification> notification = m_notificationIDMap.get(notificationID);
+ RefPtr<Notification> notification = m_nonPersistentNotifications.get(notificationID);
if (!notification)
return;
@@ -240,7 +230,7 @@
LOG(Notifications, "WebProcess %i DID CLICK notification %s", getpid(), notificationID.toString().utf8().data());
#if ENABLE(NOTIFICATIONS)
- RefPtr<Notification> notification = m_notificationIDMap.get(notificationID);
+ RefPtr<Notification> notification = m_nonPersistentNotifications.get(notificationID);
if (!notification)
return;
@@ -263,7 +253,7 @@
for (size_t i = 0; i < count; ++i) {
auto notificationID = notificationIDs[i];
- RefPtr<Notification> notification = m_notificationIDMap.take(notificationID);
+ RefPtr<Notification> notification = m_nonPersistentNotifications.take(notificationID);
if (!notification)
continue;
Modified: trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.h (292450 => 292451)
--- trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.h 2022-04-06 06:06:15 UTC (rev 292450)
+++ trunk/Source/WebKit/WebProcess/Notifications/WebNotificationManager.h 2022-04-06 07:05:25 UTC (rev 292451)
@@ -86,9 +86,7 @@
WebProcess& m_process;
#if ENABLE(NOTIFICATIONS)
- typedef HashMap<UUID, RefPtr<WebCore::Notification>> NotificationIDMap;
- NotificationIDMap m_notificationIDMap;
-
+ HashMap<UUID, Ref<WebCore::Notification>> m_nonPersistentNotifications;
HashMap<String, bool> m_permissionsMap;
#endif
};
