Modified: trunk/Source/WebCore/ChangeLog (260575 => 260576)
--- trunk/Source/WebCore/ChangeLog 2020-04-23 17:07:56 UTC (rev 260575)
+++ trunk/Source/WebCore/ChangeLog 2020-04-23 17:15:45 UTC (rev 260576)
@@ -1,3 +1,40 @@
+2020-04-23 Chris Dumez <[email protected]>
+
+ [ Mac wk2 ] imported/w3c/web-platform-tests/notifications/event-onclose.html is flaky failing.
+ https://bugs.webkit.org/show_bug.cgi?id=209483
+ <rdar://problem/60830377>
+
+ Reviewed by Geoff Garen.
+
+ Align garbage collection of Notification JS wrapper with the specification:
+ - https://notifications.spec.whatwg.org/#garbage-collection [1]
+
+ In particular, the following changes were made:
+ 1. Instead of using the legacy setPendingActivity() / unsetPendingActivity(), override
+ ActiveDOMObject::virtualHasPendingActivity() to implement the behavior documented
+ in the specification.
+ 2. Keep the wrapper alive as long as the notification is showing and as long as there
+ are relevant event listeners, as per [1]. Previously, we failed to check for event
+ listeners, which was suboptimal.
+ 3. Update the constructor to queue a task on the event loop in order to show the
+ notification asynchronously, instead of relying on a SuspendableTimer for this
+ purpose. Previously, the JS wrapper could get collected between construction and
+ the notification getting shown, which was leading to the test flakiness.
+
+ No new tests, unskipped existing test.
+
+ * Modules/notifications/Notification.cpp:
+ (WebCore::Notification::Notification):
+ (WebCore::Notification::show):
+ (WebCore::Notification::finalize):
+ (WebCore::Notification::dispatchShowEvent):
+ (WebCore::Notification::dispatchClickEvent):
+ (WebCore::Notification::dispatchCloseEvent):
+ (WebCore::Notification::dispatchErrorEvent):
+ (WebCore::Notification::eventListenersDidChange):
+ (WebCore::Notification::virtualHasPendingActivity const):
+ * Modules/notifications/Notification.h:
+
2020-04-23 Andres Gonzalez <[email protected]>
Correction for patch 397001.
Modified: trunk/Source/WebCore/Modules/notifications/Notification.cpp (260575 => 260576)
--- trunk/Source/WebCore/Modules/notifications/Notification.cpp 2020-04-23 17:07:56 UTC (rev 260575)
+++ trunk/Source/WebCore/Modules/notifications/Notification.cpp 2020-04-23 17:15:45 UTC (rev 260576)
@@ -53,6 +53,7 @@
{
auto notification = adoptRef(*new Notification(context, title, options));
notification->suspendIfNeeded();
+ notification->showSoon();
return notification;
}
@@ -64,7 +65,6 @@
, m_body(options.body)
, m_tag(options.tag)
, m_state(Idle)
- , m_showNotificationTimer(&document, *this, &Notification::show)
{
if (!options.icon.isEmpty()) {
auto iconURL = document.completeURL(options.icon);
@@ -71,13 +71,18 @@
if (iconURL.isValid())
m_icon = iconURL;
}
-
- m_showNotificationTimer.startOneShot(0_s);
- m_showNotificationTimer.suspendIfNeeded();
}
Notification::~Notification() = default;
+
+void Notification::showSoon()
+{
+ queueTaskKeepingObjectAlive(*this, TaskSource::UserInteraction, [this] {
+ show();
+ });
+}
+
void Notification::show()
{
// prevent double-showing
@@ -94,10 +99,8 @@
dispatchErrorEvent();
return;
}
- if (client.show(this)) {
+ if (client.show(this))
m_state = Showing;
- setPendingActivity(*this);
- }
}
void Notification::close()
@@ -143,28 +146,16 @@
if (m_state == Closed)
return;
m_state = Closed;
- unsetPendingActivity(*this);
}
-void Notification::queueTask(Function<void()>&& task)
-{
- auto* document = this->document();
- if (!document)
- return;
-
- document->eventLoop().queueTask(TaskSource::UserInteraction, WTFMove(task));
-}
-
void Notification::dispatchShowEvent()
{
- queueTask([this, pendingActivity = makePendingActivity(*this)] {
- dispatchEvent(Event::create(eventNames().showEvent, Event::CanBubble::No, Event::IsCancelable::No));
- });
+ queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().showEvent, Event::CanBubble::No, Event::IsCancelable::No));
}
void Notification::dispatchClickEvent()
{
- queueTask([this, pendingActivity = makePendingActivity(*this)] {
+ queueTaskKeepingObjectAlive(*this, TaskSource::UserInteraction, [this] {
WindowFocusAllowedIndicator windowFocusAllowed;
dispatchEvent(Event::create(eventNames().clickEvent, Event::CanBubble::No, Event::IsCancelable::No));
});
@@ -172,17 +163,13 @@
void Notification::dispatchCloseEvent()
{
- queueTask([this, pendingActivity = makePendingActivity(*this)] {
- dispatchEvent(Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
- });
+ queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
finalize();
}
void Notification::dispatchErrorEvent()
{
- queueTask([this, pendingActivity = makePendingActivity(*this)] {
- dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
- });
+ queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
}
auto Notification::permission(Document& document) -> Permission
@@ -215,6 +202,22 @@
NotificationController::from(page)->client().requestPermission(&document, WTFMove(callback));
}
+void Notification::eventListenersDidChange()
+{
+ m_hasRelevantEventListener = hasEventListeners(eventNames().clickEvent)
+ || hasEventListeners(eventNames().closeEvent)
+ || hasEventListeners(eventNames().errorEvent)
+ || hasEventListeners(eventNames().showEvent);
+}
+
+// A Notification object must not be garbage collected while the list of notifications contains its corresponding
+// notification and it has an event listener whose type is click, show, close, or error.
+// See https://notifications.spec.whatwg.org/#garbage-collection
+bool Notification::virtualHasPendingActivity() const
+{
+ return m_state == Showing && m_hasRelevantEventListener;
+}
+
} // namespace WebCore
#endif // ENABLE(NOTIFICATIONS)
Modified: trunk/Source/WebCore/Modules/notifications/Notification.h (260575 => 260576)
--- trunk/Source/WebCore/Modules/notifications/Notification.h 2020-04-23 17:07:56 UTC (rev 260575)
+++ trunk/Source/WebCore/Modules/notifications/Notification.h 2020-04-23 17:15:45 UTC (rev 260576)
@@ -37,7 +37,6 @@
#include "EventTarget.h"
#include "NotificationDirection.h"
#include "NotificationPermission.h"
-#include "SuspendableTimer.h"
#include <wtf/URL.h>
#include "WritingMode.h"
@@ -96,15 +95,18 @@
Document* document() const;
EventTargetInterface eventTargetInterface() const final { return NotificationEventTargetInterfaceType; }
- void queueTask(Function<void()>&&);
+ void showSoon();
// ActiveDOMObject
const char* activeDOMObjectName() const final;
void suspend(ReasonForSuspension);
void stop() final;
+ bool virtualHasPendingActivity() const final;
+ // EventTarget
void refEventTarget() final { ref(); }
void derefEventTarget() final { deref(); }
+ void eventListenersDidChange() final;
String m_title;
Direction m_direction;
@@ -115,8 +117,7 @@
enum State { Idle, Showing, Closed };
State m_state { Idle };
-
- SuspendableTimer m_showNotificationTimer;
+ bool m_hasRelevantEventListener { false };
};
} // namespace WebCore
