Implement ServiceWorkerRegistration.getNotifications() Obvious and required followup to https://bugs.webkit.org/show_bug.cgi?id=22722
Created attachment 451965 [details] EWS v1
Created attachment 451975 [details] EWS v2
Created attachment 452032 [details] EWS v3
Comment on attachment 452032 [details] EWS v3 View in context: https://bugs.webkit.org/attachment.cgi?id=452032&action=review > Source/WebCore/workers/service/ServiceWorkerRegistration.h:132 > + ListHashSet<Notification*> m_notificationList; Can this use WeakPtr?
(In reply to Alex Christensen from comment #4) > Comment on attachment 452032 [details] > EWS v3 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452032&action=review > > > Source/WebCore/workers/service/ServiceWorkerRegistration.h:132 > > + ListHashSet<Notification*> m_notificationList; > > Can this use WeakPtr? Yah that's probably better.
(In reply to Brady Eidson from comment #5) > (In reply to Alex Christensen from comment #4) > > Comment on attachment 452032 [details] > > EWS v3 > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=452032&action=review > > > > > Source/WebCore/workers/service/ServiceWorkerRegistration.h:132 > > > + ListHashSet<Notification*> m_notificationList; > > > > Can this use WeakPtr? > > Yah that's probably better. But also can't work - Can't use WeakPtr<> as a HashKey for obvious reasons.
Created attachment 452071 [details] PFR
Comment on attachment 452071 [details] PFR View in context: https://bugs.webkit.org/attachment.cgi?id=452071&action=review > Source/WebCore/Modules/notifications/Notification.cpp:88 > +Notification::Notification(const Notification& other) After calling the copy constructor, do we not need to call addNotificationToList?
(In reply to Alex Christensen from comment #8) > Comment on attachment 452071 [details] > PFR > > View in context: > https://bugs.webkit.org/attachment.cgi?id=452071&action=review > > > Source/WebCore/Modules/notifications/Notification.cpp:88 > > +Notification::Notification(const Notification& other) > > After calling the copy constructor, do we not need to call > addNotificationToList? We definitely *DON'T* want the copies to be in the list. They don't represent a live notification object the same way the ones in the list do.
Committed r289866 (247306@main): <https://commits.webkit.org/247306@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452071 [details].
<rdar://problem/88997342>