Changeset 260535 in webkit
- Timestamp:
- Apr 22, 2020 3:45:00 PM (4 years ago)
- Location:
- trunk
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r260533 r260535 1 2020-04-22 Chris Dumez <cdumez@apple.com> 2 3 [ Mac wk2 ] imported/w3c/web-platform-tests/notifications/event-onclose.html is flaky failing. 4 https://bugs.webkit.org/show_bug.cgi?id=209483 5 <rdar://problem/60830377> 6 7 Reviewed by Geoffrey Garen. 8 9 Unskip test now that it is no longer flaky. 10 11 * platform/mac-wk2/TestExpectations: 12 1 13 2020-04-22 Myles C. Maxfield <mmaxfield@apple.com> 2 14 -
trunk/LayoutTests/platform/mac-wk2/TestExpectations
r260471 r260535 1029 1029 webkit.org/b/209295 [ Debug ] imported/w3c/web-platform-tests/service-workers/service-worker/respond-with-body-accessed-response.https.html [ Pass Failure ] 1030 1030 1031 webkit.org/b/209483 imported/w3c/web-platform-tests/notifications/event-onclose.html [ Pass Failure ]1032 1033 1031 webkit.org/b/209503 [ Debug ] http/tests/referrer-policy-anchor/origin/cross-origin-http.https.html [ Pass Crash ] 1034 1032 -
trunk/Source/WebCore/ChangeLog
r260534 r260535 1 2020-04-22 Chris Dumez <cdumez@apple.com> 2 3 [ Mac wk2 ] imported/w3c/web-platform-tests/notifications/event-onclose.html is flaky failing. 4 https://bugs.webkit.org/show_bug.cgi?id=209483 5 <rdar://problem/60830377> 6 7 Reviewed by Geoffrey Garen. 8 9 Align garbage collection of Notification JS wrapper with the specification: 10 - https://notifications.spec.whatwg.org/#garbage-collection [1] 11 12 In particular, the following changes were made: 13 1. Instead of using the legacy setPendingActivity() / unsetPendingActivity(), override 14 ActiveDOMObject::virtualHasPendingActivity() to implement the behavior documented 15 in the specification. 16 2. Keep the wrapper alive as long as the notification is showing and as long as there 17 are relevant event listeners, as per [1]. Previously, we failed to check for event 18 listeners, which was suboptimal. 19 3. Update the constructor to queue a task on the event loop in order to show the 20 notification asynchronously, instead of relying on a SuspendableTimer for this 21 purpose. Previously, the JS wrapper could get collected between construction and 22 the notification getting shown, which was leading to the test flakiness. 23 24 No new tests, unskipped existing test. 25 26 * Modules/notifications/Notification.cpp: 27 (WebCore::Notification::Notification): 28 (WebCore::Notification::show): 29 (WebCore::Notification::finalize): 30 (WebCore::Notification::dispatchShowEvent): 31 (WebCore::Notification::dispatchClickEvent): 32 (WebCore::Notification::dispatchCloseEvent): 33 (WebCore::Notification::dispatchErrorEvent): 34 (WebCore::Notification::eventListenersDidChange): 35 (WebCore::Notification::virtualHasPendingActivity const): 36 * Modules/notifications/Notification.h: 37 1 38 2020-04-22 Don Olmstead <don.olmstead@sony.com> 2 39 -
trunk/Source/WebCore/Modules/notifications/Notification.cpp
r255680 r260535 65 65 , m_tag(options.tag) 66 66 , m_state(Idle) 67 , m_showNotificationTimer(&document, *this, &Notification::show)68 67 { 69 68 if (!options.icon.isEmpty()) { … … 73 72 } 74 73 75 m_showNotificationTimer.startOneShot(0_s); 76 m_showNotificationTimer.suspendIfNeeded(); 74 queueTaskKeepingObjectAlive(*this, TaskSource::UserInteraction, [this] { 75 show(); 76 }); 77 77 } 78 78 … … 95 95 return; 96 96 } 97 if (client.show(this)) {97 if (client.show(this)) 98 98 m_state = Showing; 99 setPendingActivity(*this);100 }101 99 } 102 100 … … 144 142 return; 145 143 m_state = Closed; 146 unsetPendingActivity(*this);147 }148 149 void Notification::queueTask(Function<void()>&& task)150 {151 auto* document = this->document();152 if (!document)153 return;154 155 document->eventLoop().queueTask(TaskSource::UserInteraction, WTFMove(task));156 144 } 157 145 158 146 void Notification::dispatchShowEvent() 159 147 { 160 queueTask([this, pendingActivity = makePendingActivity(*this)] { 161 dispatchEvent(Event::create(eventNames().showEvent, Event::CanBubble::No, Event::IsCancelable::No)); 162 }); 148 queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().showEvent, Event::CanBubble::No, Event::IsCancelable::No)); 163 149 } 164 150 165 151 void Notification::dispatchClickEvent() 166 152 { 167 queueTask ([this, pendingActivity = makePendingActivity(*this)] {153 queueTaskKeepingObjectAlive(*this, TaskSource::UserInteraction, [this] { 168 154 WindowFocusAllowedIndicator windowFocusAllowed; 169 155 dispatchEvent(Event::create(eventNames().clickEvent, Event::CanBubble::No, Event::IsCancelable::No)); … … 173 159 void Notification::dispatchCloseEvent() 174 160 { 175 queueTask([this, pendingActivity = makePendingActivity(*this)] { 176 dispatchEvent(Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No)); 177 }); 161 queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No)); 178 162 finalize(); 179 163 } … … 181 165 void Notification::dispatchErrorEvent() 182 166 { 183 queueTask([this, pendingActivity = makePendingActivity(*this)] { 184 dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No)); 185 }); 167 queueTaskToDispatchEvent(*this, TaskSource::UserInteraction, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No)); 186 168 } 187 169 … … 216 198 } 217 199 200 void Notification::eventListenersDidChange() 201 { 202 m_hasRelevantEventListener = hasEventListeners(eventNames().clickEvent) 203 || hasEventListeners(eventNames().closeEvent) 204 || hasEventListeners(eventNames().errorEvent) 205 || hasEventListeners(eventNames().showEvent); 206 } 207 208 // A Notification object must not be garbage collected while the list of notifications contains its corresponding 209 // notification and it has an event listener whose type is click, show, close, or error. 210 // See https://notifications.spec.whatwg.org/#garbage-collection 211 bool Notification::virtualHasPendingActivity() const 212 { 213 return m_state == Showing && m_hasRelevantEventListener; 214 } 215 218 216 } // namespace WebCore 219 217 -
trunk/Source/WebCore/Modules/notifications/Notification.h
r251528 r260535 38 38 #include "NotificationDirection.h" 39 39 #include "NotificationPermission.h" 40 #include "SuspendableTimer.h"41 40 #include <wtf/URL.h> 42 41 #include "WritingMode.h" … … 97 96 EventTargetInterface eventTargetInterface() const final { return NotificationEventTargetInterfaceType; } 98 97 99 void queueTask(Function<void()>&&);100 101 98 // ActiveDOMObject 102 99 const char* activeDOMObjectName() const final; 103 100 void suspend(ReasonForSuspension); 104 101 void stop() final; 102 bool virtualHasPendingActivity() const final; 105 103 104 // EventTarget 106 105 void refEventTarget() final { ref(); } 107 106 void derefEventTarget() final { deref(); } 107 void eventListenersDidChange() final; 108 108 109 109 String m_title; … … 116 116 enum State { Idle, Showing, Closed }; 117 117 State m_state { Idle }; 118 119 SuspendableTimer m_showNotificationTimer; 118 bool m_hasRelevantEventListener { false }; 120 119 }; 121 120
Note: See TracChangeset
for help on using the changeset viewer.