fix(snapshots): match resources by method

Previously, we only matched by url, which confuses GET and HEAD
requests where the latter is usually zero-sized.

Also make sure that resources are sorted by their monotonicTime,
since that's not always the case in the trace file, where they
are sorted by the "response body retrieved" time.

packages/trace-viewer/src/snapshotRenderer.ts

@@ -112,7 +112,7 @@ export class SnapshotRenderer {
     return { html, pageId: snapshot.pageId, frameId: snapshot.frameId, index: this._index };
   }
 
-  resourceByUrl(url: string): ResourceSnapshot | undefined {
+  resourceByUrl(url: string, method: string): ResourceSnapshot | undefined {
     const snapshot = this._snapshot;
     let result: ResourceSnapshot | undefined;
 
@@ -122,7 +122,7 @@ export class SnapshotRenderer {
         break;
       if (resource._frameref !== snapshot.frameId)
         continue;
-      if (resource.request.url === url) {
+      if (resource.request.url === url && resource.request.method === method) {
         // Pick the last resource with matching url - most likely it was used
         // at the time of snapshot, not the earlier aborted resource with the same url.
         result = resource;
@@ -134,15 +134,15 @@ export class SnapshotRenderer {
       for (const resource of this._resources) {
         if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp)
           break;
-        if (resource.request.url === url) {
+        if (resource.request.url === url && resource.request.method === method) {
           // Pick the last resource with matching url - most likely it was used
           // at the time of snapshot, not the earlier aborted resource with the same url.
           result = resource;
         }
       }
     }
 
-    if (result) {
+    if (result && method.toUpperCase() === 'GET') {
       // Patch override if necessary.
       for (const o of snapshot.resourceOverrides) {
         if (url === o.url && o.sha1) {

packages/trace-viewer/src/snapshotServer.ts

@@ -65,11 +65,11 @@ export class SnapshotServer {
     });
   }
 
-  async serveResource(requestUrlAlternatives: string[], snapshotUrl: string): Promise<Response> {
+  async serveResource(requestUrlAlternatives: string[], method: string, snapshotUrl: string): Promise<Response> {
     let resource: ResourceSnapshot | undefined;
     const snapshot = this._snapshotIds.get(snapshotUrl)!;
     for (const requestUrl of requestUrlAlternatives) {
-      resource = snapshot?.resourceByUrl(removeHash(requestUrl));
+      resource = snapshot?.resourceByUrl(removeHash(requestUrl), method);
       if (resource)
         break;
     }

packages/trace-viewer/src/snapshotStorage.ts

@@ -56,4 +56,9 @@ export class SnapshotStorage {
   snapshotsForTest() {
     return [...this._frameSnapshots.keys()];
   }
+
+  finalize() {
+    // Resources are not necessarily sorted in the trace file, so sort them now.
+    this._resources.sort((a, b) => (a._monotonicTime || 0) - (b._monotonicTime || 0));
+  }
 }

packages/trace-viewer/src/sw.ts

@@ -142,7 +142,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
   const lookupUrls = [request.url];
   if (isDeployedAsHttps && request.url.startsWith('https://'))
     lookupUrls.push(request.url.replace(/^https/, 'http'));
-  return snapshotServer.serveResource(lookupUrls, snapshotUrl);
+  return snapshotServer.serveResource(lookupUrls, request.method, snapshotUrl);
 }
 
 async function gc() {

packages/trace-viewer/src/traceModel.ts

@@ -100,6 +100,8 @@ export class TraceModel {
 
       this.contextEntries.push(contextEntry);
     }
+
+    this._snapshotStorage!.finalize();
   }
 
   async hasEntry(filename: string): Promise<boolean> {

tests/library/trace-viewer.spec.ts

@@ -871,7 +871,7 @@ test('should open trace-1.31', async ({ showTraceViewer }) => {
   await expect(snapshot.locator('[__playwright_target__]')).toHaveText(['Submit']);
 });
 
-test('should prefer later resource request', async ({ page, server, runAndTrace }) => {
+test('should prefer later resource request with the same method', async ({ page, server, runAndTrace }) => {
   const html = `
     <body>
       <script>
@@ -882,13 +882,22 @@ test('should prefer later resource request', async ({ page, server, runAndTrace
 
         if (!window.location.href.includes('reloaded'))
           window.location.href = window.location.href + '?reloaded';
+        else
+          link.onload = () => fetch('style.css', { method: 'HEAD' });
       </script>
+      <div>Hello</div>
     </body>
   `;
 
   let reloadStartedCallback = () => {};
   const reloadStartedPromise = new Promise<void>(f => reloadStartedCallback = f);
   server.setRoute('/style.css', async (req, res) => {
+    if (req.method === 'HEAD') {
+      res.statusCode = 200;
+      res.end('');
+      return;
+    }
+
     // Make sure reload happens before style arrives.
     await reloadStartedPromise;
     res.end('body { background-color: rgb(123, 123, 123) }');
@@ -900,8 +909,13 @@ test('should prefer later resource request', async ({ page, server, runAndTrace
   });
 
   const traceViewer = await runAndTrace(async () => {
+    const headRequest = page.waitForRequest(req => req.url() === server.PREFIX + '/style.css' && req.method() === 'HEAD');
     await page.goto(server.PREFIX + '/index.html');
+    await headRequest;
+    await page.locator('div').click();
   });
-  const frame = await traceViewer.snapshotFrame('page.goto');
-  await expect(frame.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)');
+  const frame1 = await traceViewer.snapshotFrame('page.goto');
+  await expect(frame1.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)');
+  const frame2 = await traceViewer.snapshotFrame('locator.click');
+  await expect(frame2.locator('body')).toHaveCSS('background-color', 'rgb(123, 123, 123)');
 });