From 26fc2c084321cbbe10c1ae38efb670620a89935b Mon Sep 17 00:00:00 2001 From: Agi Sferro Date: Tue, 25 Aug 2020 17:49:31 +0000 Subject: [PATCH] Bug 1649494 - Don't assume a notification has a URL. r=aklotz Differential Revision: https://phabricator.services.mozilla.com/D88165 --- mobile/android/geckoview/api.txt | 2 +- .../notification-test/background.js | 6 ++++ .../notification-test/manifest.json | 17 ++++++++++ .../geckoview/test/WebExtensionTest.kt | 34 +++++++++++++++++-- .../mozilla/geckoview/WebNotification.java | 7 ++-- .../mozilla/geckoview/doc-files/CHANGELOG.md | 6 +++- widget/android/AndroidAlerts.cpp | 7 ++-- 7 files changed, 69 insertions(+), 10 deletions(-) create mode 100644 mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/background.js create mode 100644 mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/manifest.json diff --git a/mobile/android/geckoview/api.txt b/mobile/android/geckoview/api.txt index 73d9bd63541e..277e8c435b60 100644 --- a/mobile/android/geckoview/api.txt +++ b/mobile/android/geckoview/api.txt @@ -1782,7 +1782,7 @@ package org.mozilla.geckoview { field @Nullable public final String imageUrl; field @Nullable public final String lang; field @NonNull public final boolean requireInteraction; - field @NonNull public final String source; + field @Nullable public final String source; field @NonNull public final String tag; field @Nullable public final String text; field @Nullable public final String textDirection; diff --git a/mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/background.js b/mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/background.js new file mode 100644 index 000000000000..463010d1d5b9 --- /dev/null +++ b/mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/background.js @@ -0,0 +1,6 @@ +browser.notifications.create("cake-notification", { + type: "basic", + title: "Time for cake!", + iconUrl: "http://example.com/img.svg", + message: "Something something cake", +}); diff --git a/mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/manifest.json b/mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/manifest.json new file mode 100644 index 000000000000..69821d8d85da --- /dev/null +++ b/mobile/android/geckoview/src/androidTest/assets/web_extensions/notification-test/manifest.json @@ -0,0 +1,17 @@ +{ + "manifest_version": 2, + "name": "Notification test", + "version": "1.0", + "description": "Send a notification.", + "browser_specific_settings": { + "gecko": { + "id": "notification@example.com" + } + }, + "background": { + "scripts": ["background.js"] + }, + "permissions": [ + "notifications" + ] +} diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt index e6b33da26578..d9eb52f074f5 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt @@ -12,7 +12,6 @@ import org.hamcrest.core.IsEqual.equalTo import org.json.JSONObject import org.junit.Assert.* import org.junit.Before -import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith import org.mozilla.geckoview.* @@ -24,8 +23,6 @@ import org.mozilla.geckoview.WebExtensionController.EnableSource import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.Setting import org.mozilla.geckoview.test.util.RuntimeCreator -import java.util.UUID - @RunWith(AndroidJUnit4::class) @MediumTest class WebExtensionTest : BaseSessionTest() { @@ -468,6 +465,37 @@ class WebExtensionTest : BaseSessionTest() { assertBodyBorderEqualTo("") } + @Test + fun createNotification() { + sessionRule.addExternalDelegateUntilTestEnd( + WebNotificationDelegate::class, + { delegate -> + sessionRule.runtime.webNotificationDelegate = delegate }, + { sessionRule.runtime.webNotificationDelegate = null }, + object : WebNotificationDelegate { + @GeckoSessionTestRule.AssertCalled + override fun onShowNotification(notification: WebNotification) { + } + }) + + val extension = sessionRule.waitForResult( + controller.installBuiltIn("resource://android/assets/web_extensions/notification-test/")) + + sessionRule.waitUntilCalled(object : WebNotificationDelegate { + @AssertCalled(count = 1) + override fun onShowNotification(notification: WebNotification) { + assertEquals(notification.title, "Time for cake!") + assertEquals(notification.text, "Something something cake") + assertEquals(notification.imageUrl, "http://example.com/img.svg") + // This should be filled out, Bug 1589693 + assertEquals(notification.source, null) + } + }) + + sessionRule.waitForResult( + controller.uninstall(extension)) + } + // This test // - Registers a web extension // - Listens for messages and waits for a message diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebNotification.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebNotification.java index b897a22733e7..90dc02851ab4 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebNotification.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebNotification.java @@ -76,8 +76,11 @@ public class WebNotification { /** * This is the URL of the page or Service Worker that generated the notification. + * Null if this notification was not generated by a Web Page (e.g. from an Extension). + * + * TODO: make NonNull once we have Bug 1589693 */ - public final @NonNull String source; + public final @Nullable String source; @WrapForJNI /* package */ WebNotification(@Nullable final String title, @NonNull final String tag, @@ -93,7 +96,7 @@ public class WebNotification { this.textDirection = textDirection; this.lang = lang; this.requireInteraction = requireInteraction; - this.source = source; + this.source = "".equals(source) ? null : source; } /** diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md index 2c84f288fbaa..e1ad5cf6d32f 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md @@ -13,6 +13,10 @@ exclude: true ⚠️ breaking change and deprecation notices +## v82 +- ⚠️ [`WebNotification.source`][79.2] is now `@Nullable` to account for + WebExtension notifications which don't have a `source` field. + ## v81 - Added `cookiePurging` to [`ContentBlocking.Settings.Builder`][81.1] and `getCookiePurging` and `setCookiePurging` to [`ContentBlocking.Settings`][81.2]. @@ -770,4 +774,4 @@ to allow adding gecko profiler markers. [65.24]: {{javadoc_uri}}/CrashReporter.html#sendCrashReport-android.content.Context-android.os.Bundle-java.lang.String- [65.25]: {{javadoc_uri}}/GeckoResult.html -[api-version]: ca995cc1f0626ab33312d27e35100485d35333a7 +[api-version]: c19f556017d98a8e4c8087ca59bde83e11aa56da diff --git a/widget/android/AndroidAlerts.cpp b/widget/android/AndroidAlerts.cpp index 3689ddb2850b..c5be998d64bf 100644 --- a/widget/android/AndroidAlerts.cpp +++ b/widget/android/AndroidAlerts.cpp @@ -80,11 +80,12 @@ AndroidAlerts::ShowPersistentNotification(const nsAString& aPersistentData, nsCOMPtr uri; rv = aAlert->GetURI(getter_AddRefs(uri)); NS_ENSURE_SUCCESS(rv, NS_OK); - MOZ_ASSERT(uri); nsCString spec; - rv = uri->GetDisplaySpec(spec); - NS_ENSURE_SUCCESS(rv, NS_OK); + if (uri) { + rv = uri->GetDisplaySpec(spec); + NS_ENSURE_SUCCESS(rv, NS_OK); + } if (aPersistentData.IsEmpty() && aAlertListener) { if (!sListenerMap) {