Skip to content

Commit

Permalink
Bug 1459953 - Part 1: Show all service worker states r=jdescottes,flu…
Browse files Browse the repository at this point in the history
…ent-reviewers,ochameau,flod

Differential Revision: https://phabricator.services.mozilla.com/D63864
  • Loading branch information
Belén Albeza committed Mar 17, 2020
1 parent 6befed7 commit fbb23a1
Show file tree
Hide file tree
Showing 21 changed files with 197 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ class ServiceWorkerAdditionalActions extends PureComponent {
className: "default-button default-button--micro qa-unregister-button",
key: "service-worker-unregister-button",
labelId: "about-debugging-worker-action-unregister",
disabled: false,
onClick: this.unregister.bind(this),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

"use strict";

const { Ci } = require("chrome");

const {
DEBUG_TARGETS,
REQUEST_WORKERS_SUCCESS,
Expand All @@ -29,14 +31,15 @@ const workerComponentDataMiddleware = store => next => action => {
};

function getServiceWorkerStatus(worker) {
const isActive = worker.active;
const isActive = worker.state === Ci.nsIServiceWorkerInfo.STATE_ACTIVATED;
const isRunning = !!worker.workerTargetFront;

if (isActive && isRunning) {
return SERVICE_WORKER_STATUSES.RUNNING;
} else if (isActive) {
return SERVICE_WORKER_STATUSES.STOPPED;
}

// We cannot get service worker registrations unless the registration is in
// ACTIVE state. Unable to know the actual state ("installing", "waiting"), we
// display a custom state "registering" for now. See Bug 1153292.
Expand Down
8 changes: 2 additions & 6 deletions devtools/client/application/initializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,8 @@ window.Application = {
},

async updateWorkers() {
const { service } = await this.client.mainRoot.listAllWorkers();
// filter out workers that don't have an URL or a scope
// TODO: Bug 1595138 investigate why we lack those properties
const workers = service.filter(x => x.url && x.scope);

this.actions.updateWorkers(workers);
const registrationsWithWorkers = await this.client.mainRoot.listAllServiceWorkers();
this.actions.updateWorkers(registrationsWithWorkers);
},

updateDomain() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,7 @@
.worker__link-debug {
margin: 0 calc(var(--base-unit) * 2);
}

.worker__status {
text-transform: capitalize;
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const {

const FluentReact = require("devtools/client/shared/vendor/fluent-react");
const Localized = createFactory(FluentReact.Localized);
const { l10n } = require("devtools/client/application/src/modules/l10n");

const {
services,
Expand Down Expand Up @@ -97,19 +98,19 @@ class Worker extends PureComponent {
}

isActive() {
return this.props.worker.active;
return this.props.worker.isActive;
}

getServiceWorkerStatus() {
getLocalizedStatus() {
if (this.isActive() && this.isRunning()) {
return "running";
return l10n.getString("serviceworker-worker-status-running");
} else if (this.isActive()) {
return "stopped";
return l10n.getString("serviceworker-worker-status-stopped");
}
// We cannot get service worker registrations unless the registration is in
// ACTIVE state. Unable to know the actual state ("installing", "waiting"), we
// display a custom state "registering" for now. See Bug 1153292.
return "registering";

// NOTE: this is already localized by the service worker front
// (strings are in debugger.properties)
return this.props.worker.stateText;
}

formatScope(scope) {
Expand Down Expand Up @@ -171,7 +172,7 @@ class Worker extends PureComponent {

render() {
const { worker } = this.props;
const status = this.getServiceWorkerStatus();
const statusText = this.getLocalizedStatus();

const unregisterButton = this.isActive()
? Localized(
Expand Down Expand Up @@ -235,10 +236,7 @@ class Worker extends PureComponent {
),
dd(
{},
Localized(
{ id: "serviceworker-worker-status-" + status },
span({ className: "js-worker-status" })
),
span({ className: "js-worker-status worker__status" }, statusText),
" ",
!this.isRunning() ? this.renderStartButton() : null
)
Expand Down
19 changes: 18 additions & 1 deletion devtools/client/application/src/reducers/workers-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

"use strict";

const { Ci } = require("chrome");

const {
UPDATE_CAN_DEBUG_WORKERS,
UPDATE_WORKERS,
Expand All @@ -17,6 +19,19 @@ function WorkersState() {
};
}

function buildWorkerDataFromFronts({ registration, workers }) {
return workers.map(worker => ({
id: worker.id,
isActive: worker.state === Ci.nsIServiceWorkerInfo.STATE_ACTIVATED,
scope: registration.scope,
lastUpdateTime: registration.lastUpdateTime, // only available for active worker
url: worker.url,
registrationFront: registration,
workerTargetFront: worker.workerTargetFront,
stateText: worker.stateText,
}));
}

function workersReducer(state = WorkersState(), action) {
switch (action.type) {
case UPDATE_CAN_DEBUG_WORKERS: {
Expand All @@ -26,7 +41,9 @@ function workersReducer(state = WorkersState(), action) {
}
case UPDATE_WORKERS: {
const { workers } = action;
return Object.assign({}, state, { list: workers });
return Object.assign({}, state, {
list: workers.map(buildWorkerDataFromFronts).flat(),
});
}
default:
return state;
Expand Down
7 changes: 4 additions & 3 deletions devtools/client/application/src/types/service-workers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
const PropTypes = require("devtools/client/shared/vendor/react-prop-types");

const worker = {
active: PropTypes.bool,
name: PropTypes.string.isRequired,
scope: PropTypes.string.isRequired,
id: PropTypes.string.isRequired,
isActive: PropTypes.bool.isRequired,
lastUpdateTime: PropTypes.number,
scope: PropTypes.string.isRequired,
stateText: PropTypes.string.isRequired,
url: PropTypes.string.isRequired,
// registrationFront can be missing in e10s.
registrationFront: PropTypes.object,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,9 @@ add_task(async function() {
const workerScript = findSource(debuggerContext, "debug-sw.js");
await removeBreakpoint(debuggerContext, workerScript.id, 11);

await unregisterAllWorkers(target.client);
await unregisterAllWorkers(target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,9 @@ add_task(async function() {
"Start button is disabled"
);

await unregisterAllWorkers(target.client);
await unregisterAllWorkers(target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ const EMPTY_URL = (URL_ROOT + "resources/service-workers/empty.html").replace(
add_task(async function() {
await enableApplicationPanel();

const { panel, toolbox } = await openNewTabAndApplicationPanel(SIMPLE_URL);
const { panel, toolbox, tab } = await openNewTabAndApplicationPanel(
SIMPLE_URL
);
const doc = panel.panelWin.document;

selectPage(panel, "service-workers");
Expand Down Expand Up @@ -58,5 +60,9 @@ add_task(async function() {
"Second service worker registration is displayed for the correct domain"
);

await unregisterAllWorkers(toolbox.target.client);
await unregisterAllWorkers(toolbox.target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ const OTHER_SCOPE_URL = URL_ROOT + "resources/service-workers/scope-page.html";
add_task(async function() {
await enableApplicationPanel();

const { panel, toolbox } = await openNewTabAndApplicationPanel(SIMPLE_URL);
const { panel, toolbox, tab } = await openNewTabAndApplicationPanel(
SIMPLE_URL
);
const doc = panel.panelWin.document;

selectPage(panel, "service-workers");
Expand Down Expand Up @@ -44,5 +46,9 @@ add_task(async function() {

ok(true, "Second service worker registration is displayed");

await unregisterAllWorkers(toolbox.target.client);
await unregisterAllWorkers(toolbox.target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,8 @@ add_task(async function() {

info("Wait until the service worker is removed from the application panel");
await waitUntil(() => getWorkerContainers(doc).length === 0);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const TAB_URL = (
add_task(async function() {
await enableApplicationPanel();

const { panel, target } = await openNewTabAndApplicationPanel(TAB_URL);
const { panel, target, tab } = await openNewTabAndApplicationPanel(TAB_URL);
const doc = panel.panelWin.document;

selectPage(panel, "service-workers");
Expand All @@ -39,5 +39,9 @@ add_task(async function() {
"Service worker has the expected Unicode url"
);

await unregisterAllWorkers(target.client);
await unregisterAllWorkers(target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ add_task(async function() {
info("Select service worker page");
selectPage(panel, "service-workers");
await waitUntil(() => doc.querySelector(".js-service-workers-page") !== null);
await unregisterAllWorkers(target.client, doc);

info("Select manifest page in the sidebar");
const link = doc.querySelector(".js-sidebar-manifest");
Expand All @@ -45,8 +46,6 @@ add_task(async function() {
await waitUntil(() => doc.querySelector(".js-manifest-page") !== null);
ok(true, "Manifest page was selected.");

await unregisterAllWorkers(target.client);

// close the tab
info("Closing the tab.");
await target.client.waitForRequestsToSettle();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,9 @@ add_task(async function() {
});
ok(true, "Worker status is 'Running'");

await unregisterAllWorkers(target.client);
await unregisterAllWorkers(target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ add_task(async function() {

// close the tab
info("Closing the tab.");
await unregisterAllWorkers(toolbox.target.client);
await unregisterAllWorkers(toolbox.target.client, doc);
await BrowserTestUtils.removeTab(tab);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ add_task(async function() {
ok(true, "Service worker list is empty");

// just in case cleanup
await unregisterAllWorkers(target.client);
await unregisterAllWorkers(target.client, doc);

// close the tab
info("Closing the tab.");
await BrowserTestUtils.removeTab(tab);
Expand Down
7 changes: 6 additions & 1 deletion devtools/client/application/test/browser/head.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ async function enableServiceWorkerDebugging() {

// Enable service workers in the debugger
await pushPref("devtools.debugger.features.windowless-service-workers", true);
// Disable randomly spawning processes during tests
await pushPref("dom.ipc.processPrelaunch.enabled", false);

// Wait for dom.ipc.processCount to be updated before releasing processes.
Services.ppmm.releaseCachedProcesses();
Expand Down Expand Up @@ -64,7 +66,7 @@ async function openNewTabAndApplicationPanel(url) {
return { panel, tab, target, toolbox };
}

async function unregisterAllWorkers(client) {
async function unregisterAllWorkers(client, doc) {
info("Wait until all workers have a valid registrationFront");
let workers;
await asyncWaitUntil(async function() {
Expand All @@ -79,6 +81,9 @@ async function unregisterAllWorkers(client) {
for (const worker of workers.service) {
await worker.registrationFront.unregister();
}

// wait for service workers to disappear from the UI
waitUntil(() => getWorkerContainers(doc).length === 0);
}

async function waitForWorkerRegistration(swTab) {
Expand Down
43 changes: 36 additions & 7 deletions devtools/client/application/test/xpcshell/test_workers_reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ http://creativecommons.org/publicdomain/zero/1.0/ */

"use strict";

const { Ci } = require("chrome");

const {
updateCanDebugWorkers,
updateWorkers,
Expand Down Expand Up @@ -33,13 +35,40 @@ add_task(async function() {

add_task(async function() {
info("Test workers reducer: UPDATE_WORKERS action");

const state = WorkersState();
const action = updateWorkers([{ foo: "bar" }, { lorem: "ipsum" }]);

const rawData = [
{
registration: {
scope: "lorem-ipsum",
lastUpdateTime: 42,
},
workers: [
{
id: 1,
state: Ci.nsIServiceWorkerInfo.STATE_ACTIVATED,
url: "https://example.com",
workerTargetFront: { foo: "bar" },
stateText: "activated",
},
],
},
];

const expectedData = [
{
id: 1,
isActive: true,
scope: "lorem-ipsum",
lastUpdateTime: 42,
url: "https://example.com",
registrationFront: rawData[0].registration,
workerTargetFront: rawData[0].workers[0].workerTargetFront,
stateText: "activated",
},
];

const action = updateWorkers(rawData);
const newState = workersReducer(state, action);
deepEqual(
newState.list,
[{ foo: "bar" }, { lorem: "ipsum" }],
"workers contains the expected list"
);
deepEqual(newState.list, expectedData, "workers contains the expected list");
});
Loading

0 comments on commit fbb23a1

Please sign in to comment.