Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/trace-viewer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"version": "0.0.0",
"type": "module",
"dependencies": {
"idb-keyval": "^6.2.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unpacked Size
928 kB

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sw.bundle.js

before
220990
after
223446

"yaml": "^2.6.0"
}
}
54 changes: 52 additions & 2 deletions packages/trace-viewer/src/sw/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

import * as idbKeyval from 'idb-keyval';

This comment was marked as resolved.

This comment was marked as resolved.


import { splitProgress } from './progress';
import { unwrapPopoutUrl } from './snapshotRenderer';
import { SnapshotServer } from './snapshotServer';
Expand All @@ -33,11 +35,14 @@ self.addEventListener('activate', function(event: any) {
});

const scopePath = new URL(self.registration.scope).pathname;

const loadedTraces = new Map<string, { traceModel: TraceModel, snapshotServer: SnapshotServer }>();

const clientIdToTraceUrls = new Map<string, { limit: number | undefined, traceUrls: Set<string>, traceViewerServer: TraceViewerServer }>();

function simulateServiceWorkerRestart() {
loadedTraces.clear();
clientIdToTraceUrls.clear();
}

async function loadTrace(traceUrl: string, traceFileName: string | null, client: any | undefined, limit: number | undefined, progress: (done: number, total: number) => undefined): Promise<TraceModel> {
await gc();
const clientId = client?.id ?? '';
Expand All @@ -49,6 +54,7 @@ async function loadTrace(traceUrl: string, traceFileName: string | null, client:
clientIdToTraceUrls.set(clientId, data);
}
data.traceUrls.add(traceUrl);
await saveClientIdParams();

const traceModel = new TraceModel();
try {
Expand Down Expand Up @@ -101,6 +107,10 @@ async function doFetch(event: FetchEvent): Promise<Response> {
await gc();
return new Response(null, { status: 200 });
}
if (relativePath === '/restartServiceWorker') {
simulateServiceWorkerRestart();
return new Response(null, { status: 200 });
}

const traceUrl = url.searchParams.get('trace');

Expand All @@ -122,6 +132,16 @@ async function doFetch(event: FetchEvent): Promise<Response> {
}
}

if (!clientIdToTraceUrls.has(event.clientId)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the 'trace/snapshot/' client this will always be true.

// Service worker was restarted upon subresource fetch.
// It was stopped because ping did not keep it alive since the tab itself was throttled.
const params = await loadClientIdParams(event.clientId);
if (params) {
for (const traceUrl of params.traceUrls)
await loadTrace(traceUrl, null, client, params.limit, () => {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadTrace also calls gc. Not really what we need here, but not sure if that can cause (concurrency) issues?

Maybe move gc out of loadTrace and just call it in the relativePath === '/contexts' branch?

}
}

if (relativePath.startsWith('/snapshotInfo/')) {
const { snapshotServer } = loadedTraces.get(traceUrl!) || {};
if (!snapshotServer)
Expand Down Expand Up @@ -221,6 +241,36 @@ async function gc() {
if (!usedTraces.has(traceUrl))
loadedTraces.delete(traceUrl);
}

await saveClientIdParams();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving here is troublesome:

When we re-open the browser tab, the service worker starts again, and the /pings requests come in, which will call gc.
At this point clientIdToTraceUrls is empty and thus we clear the database, even tough this client has traces.

So, I suggest to either only call saveClientIdParams if we call clientIdToTraceUrls.delete
and/or move the if (!clientIdToTraceUrls.has(event.clientId)) { check before the /ping handling.

}

// Persist clientIdToTraceUrls to localStorage to avoid losing it when the service worker is restarted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically IndexDB !== localStorage

async function saveClientIdParams() {
const serialized: Record<string, {
limit: number | undefined,
traceUrls: string[]
}> = {};
for (const [clientId, data] of clientIdToTraceUrls) {
serialized[clientId] = {
limit: data.limit,
traceUrls: [...data.traceUrls]
};
}

const newValue = JSON.stringify(serialized);
const oldValue = await idbKeyval.get('clientIdToTraceUrls');
if (newValue === oldValue)
return;
idbKeyval.set('clientIdToTraceUrls', newValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to await this

}

async function loadClientIdParams(clientId: string): Promise<{ limit: number | undefined, traceUrls: string[] } | undefined> {
const serialized = await idbKeyval.get('clientIdToTraceUrls') as string | undefined;
if (!serialized)
return;
const deserialized = JSON.parse(serialized);
return deserialized[clientId];
}

// @ts-ignore
Expand Down
19 changes: 19 additions & 0 deletions tests/library/trace-viewer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2021,3 +2021,22 @@ test.describe(() => {
await expect(frame.getByRole('button')).toHaveCSS('color', 'rgb(255, 0, 0)');
});
});

test('should survive service worker restart', async ({ page, runAndTrace, server }) => {
const traceViewer = await runAndTrace(async () => {
await page.goto(server.EMPTY_PAGE);
await page.setContent('Old world');
await page.evaluate(() => document.body.textContent = 'New world');
});
const snapshot1 = await traceViewer.snapshotFrame('Evaluate');
await expect(snapshot1.locator('body')).toHaveText('New world');

const status = await traceViewer.page.evaluate(async () => {
const response = await fetch('restartServiceWorker');
return response.status;
});
expect(status).toBe(200);

const snapshot2 = await traceViewer.snapshotFrame('Set content');
await expect(snapshot2.locator('body')).toHaveText('Old world');
});
Loading