For a push SW, there should be a way to do a simple push sample test from the SW listing on the about:debugging page.

See chrome://serviceworker-internals/ for an example of the simple push test. Jan, is this covered in your about:debugging features?

Yes, I think this would make sense as an advanced about:debugging feature. Thanks for filing the follow-up bug.

Brian, did you mean to block the "(null)"-title bug on some other bug? This sample-test-push feature-request seems unrelated.

(In reply to Jan Keromnes [:janx] from comment #3) > Brian, did you mean to block the "(null)"-title bug on some other bug? > > This sample-test-push feature-request seems unrelated. Yep, that was a mistake - thanks

I mean to work on this in a few weeks, self-assigning tentatively. (As always, feel free to steal if you have patches.)

This patch depends on the patches in bug 1212797. It adds a 'Push' button next to all service workers in about:debugging, regardless of whether the service workers actually subscribed to a real push service.

(I previously forgot to add the worker actor method.)

Comment on attachment 8707912 [details] [diff] [review] Add a 'Push' button for service workers in about:debugging. Review of attachment 8707912 [details] [diff] [review]: ----------------------------------------------------------------- Also need a test. ::: devtools/server/actors/worker.js @@ +139,5 @@ > + if (this._dbg.type !== Ci.nsIWorkerDebugger.TYPE_SERVICE) { > + return { error: "wrongType" }; > + } > + let registration = this._getServiceWorkerRegistrationInfo(); > + swm.sendPushEvent("", registration.scope); Do you know what happens with an empty OriginAttributes? 185 [optional_argc] void sendPushEvent(in ACString aOriginAttributes, 186 in ACString aScope, 187 [optional] in uint32_t aDataLength, 188 [optional, array, size_is(aDataLength)] in uint8_t aDataBytes); It looks like it should end up doing nothing when hitting this code: http://mxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#3184 3184 if (!swm->mRegistrationInfos.Get(aScopeKey, aData)) { As aScopeKey derivates from originAttributes and should be empty here.

[Tracking Requested - why for this release]: Part of new Service Worker & Push features that we'd like to announce with Developer Edition 47.

Tracking because this part of a new feature

(In reply to Jan Keromnes [:janx] from comment #11) > [Tracking Requested - why for this release]: Part of new Service Worker & > Push features that we'd like to announce with Developer Edition 47. Welcome back Jan! Any news on this? :)

(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #13) > (In reply to Jan Keromnes [:janx] from comment #11) > > [Tracking Requested - why for this release]: Part of new Service Worker & > > Push features that we'd like to announce with Developer Edition 47. > > Welcome back Jan! Any news on this? :) Jan?

(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #13) > (In reply to Jan Keromnes [:janx] from comment #11) > > [Tracking Requested - why for this release]: Part of new Service Worker & > > Push features that we'd like to announce with Developer Edition 47. > > Welcome back Jan! Any news on this? :) Thanks Bryan! Yes, the patch mostly works and I rebase it frequently, but it has to wait for bug 1212797 to land first, which itself has to wait for bug 1242482.

Hi Julian, could you please have a look at this patch? It adds a 'Push' button next to Service Workers in about:debugging, and you can use it to test push notifications. You can try to use it with https://simple-push-demo.appspot.com/ or https://goroost.com/try-web-push. :ochameau already pre-reviewed this patch, but it now has a test + a real originAttributes suffix. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=548c2f9f730b

Comment on attachment 8724075 [details] [diff] [review] Add a 'Push' button for service workers in about:debugging. Review of attachment 8724075 [details] [diff] [review]: ----------------------------------------------------------------- Code change looks good to me! Mostly nits about logs/comments in tests. As you can see on try, browser_service_workers_push.js fails in e10s. It can be reproduced locally, and looks like an issue/race condition with the sw-claimed message. r+ as long as the test is fixed. ::: devtools/client/aboutdebugging/components/target.js @@ +41,5 @@ > + React.createElement("button", { > + className: "push-button", > + onClick: this.push > + }, Strings.GetStringFromName("push")) : > + null), Now that Bug 1207997 has landed (sorry :) ), would be nice to rebase and use dom.button() here. ::: devtools/client/aboutdebugging/test/browser_service_workers_push.js @@ +4,5 @@ > +/* eslint-disable mozilla/no-cpows-in-tests */ > +/* global sendAsyncMessage */ > + > +"use strict"; > + Most devtools tests include a short summary as a top-level comment. Only a few about:debugging tests follow this (probably only the ones I added), so up to you to add one here. @@ +13,5 @@ > +const SERVICE_WORKER = HTTP_ROOT + "service-workers/push-sw.js"; > +const TAB_URL = HTTP_ROOT + "service-workers/push-sw.html"; > + > +add_task(function* () { > + yield new Promise(done => { Maybe add an info() before this line ? @@ +26,5 @@ > + > + let swTab = yield addTab(TAB_URL); > + > + let serviceWorkersElement = document.getElementById("service-workers"); > + yield waitForMutation(serviceWorkersElement, { childList: true }); Does not seem to fail for now, but I think it would be safer to : > // Listen for mutations. > let serviceWorkersElement = document.getElementById("service-workers"); > let onMutation = waitForMutation(serviceWorkersElement, { childList: true }); > // Add the tab containing a service worker. > let swTab = yield addTab(TAB_URL); > // Wait for the expected mutation to resolve. > yield onMutation; @@ +32,5 @@ > + // Check that the service worker appears in the UI. > + assertHasTarget(true, document, "service-workers", SERVICE_WORKER); > + > + // Ensure that the registration resolved before trying to interact with the > + // service worker. This comment could be turned into an info() (makes logs more useful when investigating test failures) @@ +44,5 @@ > + let targetElement = name.parentNode.parentNode; > + let pushBtn = targetElement.querySelector(".push-button"); > + ok(pushBtn, "Found its push button"); > + > + // Make the test page notify us when the service worker sends a message. This comment would be a useful info() @@ +55,5 @@ > + }; > + let mm = swTab.linkedBrowser.messageManager; > + mm.loadFrameScript("data:,(" + encodeURIComponent(frameScript) + ")()", true); > + > + // Wait for the service worker to claim the test window before proceeding. This comment would be a useful info() @@ +57,5 @@ > + mm.loadFrameScript("data:,(" + encodeURIComponent(frameScript) + ")()", true); > + > + // Wait for the service worker to claim the test window before proceeding. > + yield new Promise(done => { > + mm.addMessageListener("sw-claimed", function listener() { Test times out on e10s because "sw-claimed" is never received. FWIW, moving the listener right after `let swTab = yield addTab(TAB_URL);` fixes the issue, but I didn't check if the test was still valid when doing that. @@ +63,5 @@ > + done(); > + }); > + }); > + > + // Click on the Push button and wait for the service worker to receive a push This comment would be a useful info() @@ +75,5 @@ > + pushBtn.click(); > + yield onPushNotification; > + ok(true, "Service worker received a push notification"); > + > + // Finally, unregister the service worker itself. This comment would be a useful info() ::: devtools/client/aboutdebugging/test/head.js @@ +149,5 @@ > observer.observe(target, mutationOptions); > }); > } > + > +function assertHasTarget(expected, document, type, name) { Can you add a JS doc for this method ? @@ +156,5 @@ > + is(names.includes(name), expected, > + "The " + type + " url appears in the list: " + names); > +} > + > +function waitForServiceWorkerRegistered(tab) { JS doc @@ +159,5 @@ > + > +function waitForServiceWorkerRegistered(tab) { > + // Make the test page notify us when the service worker is registered. > + let frameScript = function() { > + // Retrieve the `sw` promise created in the html page nit: punctuation : dot at the end of line. @@ +176,5 @@ > + }); > + }); > +} > + > +function unregisterServiceWorker(tab) { JS doc

Rebased, addressed nits.

Comment on attachment 8724669 [details] [diff] [review] Add a 'Push' button for service workers in about:debugging. Carrying over r+. Julian, could you please send this patch to try for me? I have configuration issues that will take some time to resolve. try: -b do -p linux,linux64,macosx64,win64 -u reftest,reftest-e10s,mochitest-dt,mochitest-e10s-dt -t none

try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb7f28659d2a

Thanks Julian! Try push looks green to me.

Release Note Request (optional, but appreciated) [Why is this notable]: Part of new Devtools for Service Workers, adds a button to trigger a "push" event on a SW in about:debugging#workers [Suggested wording]: [Links (documentation, blog post, etc)]:

dev-doc-needed: The relevant MDN page is here https://developer.mozilla.org/en-US/docs/Tools/about:debugging

Added to Fx47 (Aurora) release notes.

This is done: https://developer.mozilla.org/en-US/docs/Tools/about:debugging