Diff
Modified: trunk/LayoutTests/ChangeLog (251527 => 251528)
--- trunk/LayoutTests/ChangeLog 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/LayoutTests/ChangeLog 2019-10-24 04:01:30 UTC (rev 251528)
@@ -1,3 +1,18 @@
+2019-10-23 Chris Dumez <cdu...@apple.com>
+
+ Notification should not prevent entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=203099
+ <rdar://problem/56557479>
+
+ Reviewed by Geoffrey Garen.
+
+ Add layout test coverage.
+
+ * fast/history/page-cache-notification-non-suspendable-expected.txt: Removed.
+ * fast/history/page-cache-notification-non-suspendable.html: Removed.
+ * fast/history/page-cache-notification-showing-expected.txt: Added.
+ * fast/history/page-cache-notification-showing.html: Added.
+
2019-10-23 Said Abou-Hallawa <sabouhall...@apple.com>
[SVG2] Fix SVGSVGElement to conform with SVG2
Deleted: trunk/LayoutTests/fast/history/page-cache-notification-non-suspendable-expected.txt (251527 => 251528)
--- trunk/LayoutTests/fast/history/page-cache-notification-non-suspendable-expected.txt 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/LayoutTests/fast/history/page-cache-notification-non-suspendable-expected.txt 2019-10-24 04:01:30 UTC (rev 251528)
@@ -1,11 +0,0 @@
-Tests that a page with a notification that is showing does not enter page cache.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-pageshow - not from cache
-PASS Page was not restored from page cache
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
Deleted: trunk/LayoutTests/fast/history/page-cache-notification-non-suspendable.html (251527 => 251528)
--- trunk/LayoutTests/fast/history/page-cache-notification-non-suspendable.html 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/LayoutTests/fast/history/page-cache-notification-non-suspendable.html 2019-10-24 04:01:30 UTC (rev 251528)
@@ -1,53 +0,0 @@
-<!-- webkit-test-runner [ enableBackForwardCache=true ] -->
-<!DOCTYPE html>
-<html>
-<body>
-<script src=""
-<script>
-description('Tests that a page with a notification that is showing does not enter page cache.');
-window.jsTestIsAsync = true;
-
-if (window.testRunner)
- testRunner.grantWebNotificationPermission("file://");
-
-window.addEventListener("pageshow", function(event) {
- debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
- if (!window.sessionStorage.page_cache_notifications_test_started)
- return;
-
- delete window.sessionStorage.page_cache_notifications_test_started;
-
- if (event.persisted)
- testFailed("Page did enter and was restored from the page cache");
- else
- testPassed("Page was not restored from page cache");
- finishJSTest();
-}, false);
-
-window.addEventListener("pagehide", function(event) {
- debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
- if (event.persisted) {
- testFailed("Page entered the page cache.");
- finishJSTest();
- }
-}, false);
-
-window.addEventListener('load', function() {
- window.sessionStorage.page_cache_notifications_test_started = true;
-
- // This notification is shown and should not be suspendable.
- notification = new Notification('title', { body: 'body' });
- notification._onerror_ = function() {
- testFailed("Could not show the notification");
- finishJSTest();
- };
- notification._ondisplay_ = function() {
- // Force a back navigation back to this page.
- window.location.href = ""
- };
-}, false);
-
-</script>
-<script src=""
-</body>
-</html>
Added: trunk/LayoutTests/fast/history/page-cache-notification-showing-expected.txt (0 => 251528)
--- trunk/LayoutTests/fast/history/page-cache-notification-showing-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/history/page-cache-notification-showing-expected.txt 2019-10-24 04:01:30 UTC (rev 251528)
@@ -0,0 +1,15 @@
+Tests that a page with a notification that is showing is able to enter the back/forward cache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+pageshow - not from cache
+pagehide - entering cache
+pageshow - from cache
+PASS Page did enter and was restored from the back/forward cache
+PASS Received closed notification
+PASS restoredFromCache is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/history/page-cache-notification-showing.html (0 => 251528)
--- trunk/LayoutTests/fast/history/page-cache-notification-showing.html (rev 0)
+++ trunk/LayoutTests/fast/history/page-cache-notification-showing.html 2019-10-24 04:01:30 UTC (rev 251528)
@@ -0,0 +1,50 @@
+<!-- webkit-test-runner [ enableBackForwardCache=true ] -->
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description('Tests that a page with a notification that is showing is able to enter the back/forward cache.');
+jsTestIsAsync = true;
+restoredFromCache = false;
+
+if (window.testRunner)
+ testRunner.grantWebNotificationPermission("file://");
+
+window.addEventListener("pageshow", function(event) {
+ debug("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+
+ if (event.persisted) {
+ testPassed("Page did enter and was restored from the back/forward cache");
+ restoredFromCache = true;
+ }
+}, false);
+
+window.addEventListener("pagehide", function(event) {
+ debug("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ testFailed("Page failed to enter the back/forward cache.");
+ finishJSTest();
+ }
+}, false);
+
+window.addEventListener('load', function() {
+ setTimeout(() => {
+ notification = new Notification('title', { body: 'body' });
+ notification._onerror_ = () => {
+ testFailed("Could not show the notification");
+ finishJSTest();
+ };
+ notification._onclose_ = () => {
+ testPassed("Received closed notification");
+ shouldBeTrue("restoredFromCache");
+ finishJSTest();
+ };
+ notification._ondisplay_ = () => {
+ window.location.href = ""
+ };
+ }, 0);
+}, false);
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (251527 => 251528)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2019-10-24 04:01:30 UTC (rev 251528)
@@ -174,7 +174,7 @@
webkit.org/b/98927 fast/dom/DeviceOrientation [ Skip ]
# https://bugs.webkit.org/show_bug.cgi?id=81697 Skip file:// based notifications tests
-fast/history/page-cache-notification-non-suspendable.html
+fast/history/page-cache-notification-showing.html
fast/history/page-cache-notification-suspendable.html
# ENABLE_INPUT_TYPE_* are not enabled.
Modified: trunk/LayoutTests/platform/ios/TestExpectations (251527 => 251528)
--- trunk/LayoutTests/platform/ios/TestExpectations 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2019-10-24 04:01:30 UTC (rev 251528)
@@ -57,7 +57,7 @@
# Web Notifications are not supported on iOS.
http/tests/notifications
-fast/history/page-cache-notification-non-suspendable.html
+fast/history/page-cache-notification-showing.html
fast/history/page-cache-notification-suspendable.html
imported/w3c/web-platform-tests/notifications/
Modified: trunk/LayoutTests/platform/win/TestExpectations (251527 => 251528)
--- trunk/LayoutTests/platform/win/TestExpectations 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/LayoutTests/platform/win/TestExpectations 2019-10-24 04:01:30 UTC (rev 251528)
@@ -195,7 +195,7 @@
# TODO This port doesn't support Notifications.
http/tests/notifications/ [ Skip ]
-fast/history/page-cache-notification-non-suspendable.html [ Skip ]
+fast/history/page-cache-notification-showing.html [ Skip ]
fast/history/page-cache-notification-suspendable.html [ Skip ]
# TODO No support for print-to-pdf in Windows DRT
Modified: trunk/Source/WebCore/ChangeLog (251527 => 251528)
--- trunk/Source/WebCore/ChangeLog 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/Source/WebCore/ChangeLog 2019-10-24 04:01:30 UTC (rev 251528)
@@ -1,3 +1,35 @@
+2019-10-23 Chris Dumez <cdu...@apple.com>
+
+ Notification should not prevent entering the back/forward cache
+ https://bugs.webkit.org/show_bug.cgi?id=203099
+ <rdar://problem/56557479>
+
+ Reviewed by Geoffrey Garen.
+
+ Notifications currently displayed were preventing a page from entering the back/forward cache.
+ Instead, we now make sure to close any visible notification upon suspension. Since closing
+ a notification will fire events, we also schedule tasks on the window event loop to fire those
+ events, instead of firing these events synchronously. The Window event loop takes care of delaying
+ those events if the document is suspended.
+
+ Test: fast/history/page-cache-notification-showing.html
+
+ * Modules/notifications/Notification.cpp:
+ (WebCore::Notification::Notification):
+ (WebCore::Notification::show):
+ (WebCore::Notification::close):
+ (WebCore::Notification::document const):
+ (WebCore::Notification::activeDOMObjectName const):
+ (WebCore::Notification::stop):
+ (WebCore::Notification::suspend):
+ (WebCore::Notification::queueTask):
+ (WebCore::Notification::dispatchShowEvent):
+ (WebCore::Notification::dispatchClickEvent):
+ (WebCore::Notification::dispatchCloseEvent):
+ (WebCore::Notification::dispatchErrorEvent):
+ * Modules/notifications/Notification.h:
+ * dom/AbstractEventLoop.h:
+
2019-10-23 Said Abou-Hallawa <sabouhall...@apple.com>
[SVG2] Fix SVGSVGElement to conform with SVG2
Modified: trunk/Source/WebCore/Modules/notifications/Notification.cpp (251527 => 251528)
--- trunk/Source/WebCore/Modules/notifications/Notification.cpp 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/Source/WebCore/Modules/notifications/Notification.cpp 2019-10-24 04:01:30 UTC (rev 251528)
@@ -41,6 +41,7 @@
#include "NotificationClient.h"
#include "NotificationController.h"
#include "NotificationPermissionCallback.h"
+#include "WindowEventLoop.h"
#include "WindowFocusAllowedIndicator.h"
#include <wtf/IsoMallocInlines.h>
@@ -63,7 +64,7 @@
, m_body(options.body)
, m_tag(options.tag)
, m_state(Idle)
- , m_taskTimer(makeUnique<Timer>([this] () { show(); }))
+ , m_showNotificationTimer(&document, *this, &Notification::show)
{
if (!options.icon.isEmpty()) {
auto iconURL = document.completeURL(options.icon);
@@ -71,7 +72,8 @@
m_icon = iconURL;
}
- m_taskTimer->startOneShot(0_s);
+ m_showNotificationTimer.startOneShot(0_s);
+ m_showNotificationTimer.suspendIfNeeded();
}
Notification::~Notification() = default;
@@ -82,7 +84,7 @@
if (m_state != Idle)
return;
- auto* page = downcast<Document>(*scriptExecutionContext()).page();
+ auto* page = document()->page();
if (!page)
return;
@@ -104,8 +106,7 @@
case Idle:
break;
case Showing: {
- auto* page = downcast<Document>(*scriptExecutionContext()).page();
- if (page)
+ if (auto* page = document()->page())
NotificationController::from(page)->client().cancel(this);
break;
}
@@ -114,15 +115,14 @@
}
}
-const char* Notification::activeDOMObjectName() const
+Document* Notification::document() const
{
- return "Notification";
+ return downcast<Document>(scriptExecutionContext());
}
-// FIXME: This should never prevent entering the back/forward cache.
-bool Notification::shouldPreventEnteringBackForwardCache_DEPRECATED() const
+const char* Notification::activeDOMObjectName() const
{
- return m_state != Idle && m_state != Closed;
+ return "Notification";
}
void Notification::stop()
@@ -129,11 +129,15 @@
{
ActiveDOMObject::stop();
- auto* page = downcast<Document>(*scriptExecutionContext()).page();
- if (page)
+ if (auto* page = document()->page())
NotificationController::from(page)->client().notificationObjectDestroyed(this);
}
+void Notification::suspend(ReasonForSuspension)
+{
+ close();
+}
+
void Notification::finalize()
{
if (m_state == Closed)
@@ -142,26 +146,43 @@
unsetPendingActivity(*this);
}
+void Notification::queueTask(Function<void()>&& task)
+{
+ auto* document = this->document();
+ if (!document)
+ return;
+
+ document->eventLoop().queueTask(TaskSource::UserInteraction, *document, WTFMove(task));
+}
+
void Notification::dispatchShowEvent()
{
- dispatchEvent(Event::create(eventNames().showEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ queueTask([this, pendingActivity = makePendingActivity(*this)] {
+ dispatchEvent(Event::create(eventNames().showEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ });
}
void Notification::dispatchClickEvent()
{
- WindowFocusAllowedIndicator windowFocusAllowed;
- dispatchEvent(Event::create(eventNames().clickEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ queueTask([this, pendingActivity = makePendingActivity(*this)] {
+ WindowFocusAllowedIndicator windowFocusAllowed;
+ dispatchEvent(Event::create(eventNames().clickEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ });
}
void Notification::dispatchCloseEvent()
{
- dispatchEvent(Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ queueTask([this, pendingActivity = makePendingActivity(*this)] {
+ dispatchEvent(Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ });
finalize();
}
void Notification::dispatchErrorEvent()
{
- dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ queueTask([this, pendingActivity = makePendingActivity(*this)] {
+ dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ });
}
auto Notification::permission(Document& document) -> Permission
Modified: trunk/Source/WebCore/Modules/notifications/Notification.h (251527 => 251528)
--- trunk/Source/WebCore/Modules/notifications/Notification.h 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/Source/WebCore/Modules/notifications/Notification.h 2019-10-24 04:01:30 UTC (rev 251528)
@@ -37,7 +37,7 @@
#include "EventTarget.h"
#include "NotificationDirection.h"
#include "NotificationPermission.h"
-#include "Timer.h"
+#include "SuspendableTimer.h"
#include <wtf/URL.h>
#include "WritingMode.h"
@@ -93,11 +93,14 @@
private:
Notification(Document&, const String& title, const Options&);
+ Document* document() const;
EventTargetInterface eventTargetInterface() const final { return NotificationEventTargetInterfaceType; }
+ void queueTask(Function<void()>&&);
+
// ActiveDOMObject
const char* activeDOMObjectName() const final;
- bool shouldPreventEnteringBackForwardCache_DEPRECATED() const final;
+ void suspend(ReasonForSuspension);
void stop() final;
void refEventTarget() final { ref(); }
@@ -113,7 +116,7 @@
enum State { Idle, Showing, Closed };
State m_state { Idle };
- std::unique_ptr<Timer> m_taskTimer;
+ SuspendableTimer m_showNotificationTimer;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/AbstractEventLoop.h (251527 => 251528)
--- trunk/Source/WebCore/dom/AbstractEventLoop.h 2019-10-24 03:42:59 UTC (rev 251527)
+++ trunk/Source/WebCore/dom/AbstractEventLoop.h 2019-10-24 04:01:30 UTC (rev 251528)
@@ -35,6 +35,7 @@
enum class TaskSource : uint8_t {
IdleTask,
Networking,
+ UserInteraction
};
// https://html.spec.whatwg.org/multipage/webappapis.html#event-loop
