cherry-pick(#24145): fix(snapshots): match resources by method

packages/playwright-core/src/server/har/harTracer.ts

@@ -437,6 +437,12 @@ export class HarTracer {
     const pageEntry = this._createPageEntryIfNeeded(page);
     const request = response.request();
 
+    // Prefer "response received" time over "request sent" time
+    // for the purpose of matching requests that were used in a particular snapshot.
+    // Note that both snapshot time and request time are taken here in the Node process.
+    if (this._options.includeTraceInfo)
+      harEntry._monotonicTime = monotonicTime();
+
     harEntry.response = {
       status: response.status(),
       statusText: response.statusText(),

packages/trace-viewer/src/snapshotRenderer.ts

@@ -112,17 +112,19 @@ 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;
 
     // First try locating exact resource belonging to this frame.
     for (const resource of this._resources) {
+      // Only use resources that received response before the snapshot.
+      // Note that both snapshot time and request time are taken in the same Node process.
       if (typeof resource._monotonicTime === 'number' && resource._monotonicTime >= snapshot.timestamp)
         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;
@@ -132,17 +134,19 @@ export class SnapshotRenderer {
     if (!result) {
       // Then fall back to resource with this URL to account for memory cache.
       for (const resource of this._resources) {
+        // Only use resources that received response before the snapshot.
+        // Note that both snapshot time and request time are taken in the same Node process.
         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/snapshotter.spec.ts

@@ -51,7 +51,7 @@ it.describe('snapshots', () => {
     });
     await page.setContent('<link rel="stylesheet" href="style.css"><button>Hello</button>');
     const snapshot = await snapshotter.captureSnapshot(toImpl(page), 'call@1', 'snapshot@call@1');
-    const resource = snapshot.resourceByUrl(`http://localhost:${server.PORT}/style.css`);
+    const resource = snapshot.resourceByUrl(`http://localhost:${server.PORT}/style.css`, 'GET');
     expect(resource).toBeTruthy();
   });
 
@@ -124,7 +124,7 @@ it.describe('snapshots', () => {
 
     await page.evaluate(() => { (document.styleSheets[0].cssRules[0] as any).style.color = 'blue'; });
     const snapshot2 = await snapshotter.captureSnapshot(toImpl(page), 'call@2', 'snapshot@call@2');
-    const resource = snapshot2.resourceByUrl(`http://localhost:${server.PORT}/style.css`);
+    const resource = snapshot2.resourceByUrl(`http://localhost:${server.PORT}/style.css`, 'GET');
     expect((await snapshotter.resourceContentForTest(resource.response.content._sha1)).toString()).toBe('button { color: blue; }');
   });
 

tests/library/trace-viewer.spec.ts

@@ -851,7 +851,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>
@@ -862,13 +862,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) }');
@@ -880,8 +889,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)');
 });