diff --git a/dom/notification/Notification.cpp b/dom/notification/Notification.cpp index 2205387cefb5..39ba7b23fffa 100644 --- a/dom/notification/Notification.cpp +++ b/dom/notification/Notification.cpp @@ -714,12 +714,7 @@ Notification::Notification(nsIGlobalObject* aGlobal, const nsAString& aID, } } -nsresult Notification::MaybeObserveWindowFrozenOrDestroyed() { - // NOTE: Non-persistent notifications can also be opened from workers, but we - // don't care and nobody else cares. And it's not clear whether we even should - // do this for window at all, see - // https://github.com/whatwg/notifications/issues/204. - // TODO: Somehow extend GlobalTeardownObserver to deal with FROZEN_TOPIC? +nsresult Notification::Init() { if (!mWorkerPrivate) { nsCOMPtr obs = mozilla::services::GetObserverService(); NS_ENSURE_TRUE(obs, NS_ERROR_FAILURE); @@ -781,10 +776,6 @@ already_AddRefed Notification::Constructor( if (NS_WARN_IF(aRv.Failed())) { return nullptr; } - if (NS_WARN_IF( - NS_FAILED(notification->MaybeObserveWindowFrozenOrDestroyed()))) { - return nullptr; - } // This is be ok since we are on the worker thread where this function will // run to completion before the Notification has a chance to go away. @@ -924,6 +915,8 @@ already_AddRefed Notification::CreateInternal( aGlobal, id, aTitle, aOptions.mBody, aOptions.mDir, aOptions.mLang, aOptions.mTag, aOptions.mIcon, aOptions.mRequireInteraction, silent, std::move(vibrate), aOptions.mMozbehavior); + rv = notification->Init(); + NS_ENSURE_SUCCESS(rv, nullptr); return notification.forget(); } @@ -1316,8 +1309,6 @@ bool Notification::IsInPrivateBrowsing() { return false; } -// Step 4 of -// https://notifications.spec.whatwg.org/#dom-notification-notification void Notification::ShowInternal() { AssertIsOnMainThread(); MOZ_ASSERT(mTempRef, @@ -1332,15 +1323,13 @@ void Notification::ShowInternal() { std::swap(ownership, mTempRef); MOZ_ASSERT(ownership->GetNotification() == this); + nsresult rv = PersistNotification(); + if (NS_FAILED(rv)) { + NS_WARNING("Could not persist Notification"); + } + nsCOMPtr alertService = components::Alerts::Service(); - // Step 4.1: If the result of getting the notifications permission state is - // not "granted", then queue a task to fire an event named error on this, and - // abort these steps. - // - // XXX: But this function is also triggered by - // Notification::ShowPersistentNotification which already does its own - // permission check. Can we split this? ErrorResult result; NotificationPermission permission = NotificationPermission::Denied; if (mWorkerPrivate) { @@ -1361,28 +1350,13 @@ void Notification::ShowInternal() { } else { DispatchTrustedEvent(u"error"_ns); } - mIsClosed = true; return; } - // Preparing for Step 4.2 the fetch steps. The actual work happens in - // nsIAlertNotification::LoadImage nsAutoString iconUrl; nsAutoString soundUrl; ResolveIconAndSoundURL(iconUrl, soundUrl); - // Step 4.3 the show steps, which are almost all about processing `tag` and - // then displaying the notification. Both are handled by - // nsIAlertsService::ShowAlert/PersistentNotification. The below is all about - // constructing the observer (for show and close events) right and ultimately - // call the alerts service function. - - // XXX: Non-persistent notifications probably don't need this - nsresult rv = PersistNotification(); - if (NS_FAILED(rv)) { - NS_WARNING("Could not persist Notification"); - } - bool isPersistent = false; nsCOMPtr observer; if (mScope.IsEmpty()) { diff --git a/dom/notification/Notification.h b/dom/notification/Notification.h index 61f4a8f1c3e7..4ffa69cf49f5 100644 --- a/dom/notification/Notification.h +++ b/dom/notification/Notification.h @@ -245,8 +245,7 @@ class Notification : public DOMEventTargetHelper, nsIGlobalObject* aGlobal, const nsAString& aID, const nsAString& aTitle, const NotificationOptions& aOptions, ErrorResult& aRv); - // Triggers CloseInternal for non-persistent notifications if window goes away - nsresult MaybeObserveWindowFrozenOrDestroyed(); + nsresult Init(); bool IsInPrivateBrowsing(); void ShowInternal(); void CloseInternal(bool aContextClosed = false); diff --git a/testing/web-platform/meta/notifications/shownotification-window.https.html.ini b/testing/web-platform/meta/notifications/shownotification-window.https.html.ini deleted file mode 100644 index 896b997c36ec..000000000000 --- a/testing/web-platform/meta/notifications/shownotification-window.https.html.ini +++ /dev/null @@ -1 +0,0 @@ -prefs: [notification.prompt.testing:true,marionette.setpermission.enabled:true] diff --git a/testing/web-platform/tests/notifications/global-teardown-crash.html b/testing/web-platform/tests/notifications/global-teardown-crash.html deleted file mode 100644 index 27db2ca3b60d..000000000000 --- a/testing/web-platform/tests/notifications/global-teardown-crash.html +++ /dev/null @@ -1,16 +0,0 @@ - - - - diff --git a/testing/web-platform/tests/notifications/resources/helpers.js b/testing/web-platform/tests/notifications/resources/helpers.js index ca44e32f7f49..8c30173336ee 100644 --- a/testing/web-platform/tests/notifications/resources/helpers.js +++ b/testing/web-platform/tests/notifications/resources/helpers.js @@ -12,24 +12,9 @@ async function getActiveServiceWorker(script) { return reg; } + async function closeAllNotifications() { for (const n of await registration.getNotifications()) { n.close(); } } - -async function trySettingPermission(perm) { - try { - await test_driver.set_permission({ name: "notifications" }, perm); - } catch { - // Not all implementations support this yet, but the permission may already be set to be able to continue - } - - // Using Notification.permission instead of permissions.query() as - // some implementation without set_permission support overrides - // Notification.permission. - const permission = Notification.permission === "default" ? "prompt" : Notification.permission; - if (permission !== perm) { - throw new Error(`Should have the permission ${perm} to continue`); - } -} diff --git a/testing/web-platform/tests/notifications/resources/shownotification-window-iframe.html b/testing/web-platform/tests/notifications/resources/shownotification-window-iframe.html deleted file mode 100644 index 2a45e794656f..000000000000 --- a/testing/web-platform/tests/notifications/resources/shownotification-window-iframe.html +++ /dev/null @@ -1,8 +0,0 @@ - - - diff --git a/testing/web-platform/tests/notifications/shownotification-window.https.html b/testing/web-platform/tests/notifications/shownotification-window.https.html deleted file mode 100644 index b21a5621df75..000000000000 --- a/testing/web-platform/tests/notifications/shownotification-window.https.html +++ /dev/null @@ -1,37 +0,0 @@ - - - - - - - - - - diff --git a/testing/web-platform/tests/notifications/shownotification-without-permission.https.window.js b/testing/web-platform/tests/notifications/shownotification-without-permission.https.window.js index b09c0460fbd5..37b3dbbef662 100644 --- a/testing/web-platform/tests/notifications/shownotification-without-permission.https.window.js +++ b/testing/web-platform/tests/notifications/shownotification-without-permission.https.window.js @@ -8,14 +8,20 @@ let registration; promise_setup(async () => { - await trySettingPermission("prompt"); registration = await getActiveServiceWorker("noop-sw.js"); - await closeAllNotifications(); }); -promise_test(async t => { +promise_test(async (t) => { t.add_cleanup(closeAllNotifications); + try { + await test_driver.set_permission({ name: "notifications" }, "prompt"); + } catch { + // Not all implementations support this yet, but it may already be "prompt" to be able to continue + } + + assert_equals(Notification.permission, "default", "Should have the default permission to continue"); + await promise_rejects_js(t, TypeError, registration.showNotification(""), "Should throw TypeError"); const notifications = await registration.getNotifications(); assert_equals(notifications.length, 0, "Should return zero notification"); diff --git a/widget/windows/ToastNotification.cpp b/widget/windows/ToastNotification.cpp index 539d24abe923..afdffc19acbe 100644 --- a/widget/windows/ToastNotification.cpp +++ b/widget/windows/ToastNotification.cpp @@ -776,8 +776,6 @@ ToastNotification::CloseAlert(const nsAString& aAlertName, bool aContextClosed) { RefPtr handler; if (NS_WARN_IF(!mActiveHandlers.Get(aAlertName, getter_AddRefs(handler)))) { - MOZ_ASSERT_UNREACHABLE( - "Closing a notification that is not being handled by nsIAlertsService"); return NS_OK; }