From 295d0a65c33580aa5e9077866161aace840ae2f2 Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Fri, 21 Jan 2022 16:31:00 -0800
Subject: [PATCH] fix(har): do not hang on cached resources (#11556)

---
 .../src/server/chromium/crNetworkManager.ts   | 19 +++++++++++++----
 .../src/server/supplements/har/harTracer.ts   |  1 +
 tests/har.spec.ts                             | 21 +++++++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts
index fd5f1e3a8b..5cb6b93659 100644
--- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts
+++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts
@@ -53,6 +53,7 @@ export class CRNetworkManager {
       eventsHelper.addEventListener(session, 'Fetch.authRequired', this._onAuthRequired.bind(this)),
       eventsHelper.addEventListener(session, 'Network.requestWillBeSent', this._onRequestWillBeSent.bind(this, workerFrame)),
       eventsHelper.addEventListener(session, 'Network.requestWillBeSentExtraInfo', this._onRequestWillBeSentExtraInfo.bind(this)),
+      eventsHelper.addEventListener(session, 'Network.requestServedFromCache', this._onRequestServedFromCache.bind(this)),
       eventsHelper.addEventListener(session, 'Network.responseReceived', this._onResponseReceived.bind(this)),
       eventsHelper.addEventListener(session, 'Network.responseReceivedExtraInfo', this._onResponseReceivedExtraInfo.bind(this)),
       eventsHelper.addEventListener(session, 'Network.loadingFinished', this._onLoadingFinished.bind(this)),
@@ -134,6 +135,10 @@ export class CRNetworkManager {
     }
   }
 
+  _onRequestServedFromCache(event: Protocol.Network.requestServedFromCachePayload) {
+    this._responseExtraInfoTracker.requestServedFromCache(event);
+  }
+
   _onRequestWillBeSentExtraInfo(event: Protocol.Network.requestWillBeSentExtraInfoPayload) {
     this._responseExtraInfoTracker.requestWillBeSentExtraInfo(event);
   }
@@ -556,6 +561,7 @@ type RequestInfo = {
   loadingFinished?: Protocol.Network.loadingFinishedPayload,
   loadingFailed?: Protocol.Network.loadingFailedPayload,
   sawResponseWithoutConnectionId: boolean
+  requestServedFromCache: boolean;
 };
 
 // This class aligns responses with response headers from extra info:
@@ -585,12 +591,15 @@ class ResponseExtraInfoTracker {
 
   requestWillBeSentExtraInfo(event: Protocol.Network.requestWillBeSentExtraInfoPayload) {
     const info = this._getOrCreateEntry(event.requestId);
-    if (!info)
-      return;
     info.requestWillBeSentExtraInfo.push(event);
     this._patchHeaders(info, info.requestWillBeSentExtraInfo.length - 1);
   }
 
+  requestServedFromCache(event: Protocol.Network.requestServedFromCachePayload) {
+    const info = this._getOrCreateEntry(event.requestId);
+    info.requestServedFromCache = true;
+  }
+
   responseReceived(event: Protocol.Network.responseReceivedPayload) {
     const info = this._requests.get(event.requestId);
     if (!info)
@@ -630,7 +639,8 @@ class ResponseExtraInfoTracker {
     const info = this._requests.get(requestId);
     if (!info || info.sawResponseWithoutConnectionId)
       return;
-    response.setWillReceiveExtraHeaders();
+    if (!info.requestServedFromCache)
+      response.setWillReceiveExtraHeaders();
     info.responses.push(response);
     this._patchHeaders(info, info.responses.length - 1);
   }
@@ -659,7 +669,8 @@ class ResponseExtraInfoTracker {
         requestWillBeSentExtraInfo: [],
         responseReceivedExtraInfo: [],
         responses: [],
-        sawResponseWithoutConnectionId: false
+        sawResponseWithoutConnectionId: false,
+        requestServedFromCache: false
       };
       this._requests.set(requestId, info);
     }
diff --git a/packages/playwright-core/src/server/supplements/har/harTracer.ts b/packages/playwright-core/src/server/supplements/har/harTracer.ts
index 4233f1744b..b6936574bf 100644
--- a/packages/playwright-core/src/server/supplements/har/harTracer.ts
+++ b/packages/playwright-core/src/server/supplements/har/harTracer.ts
@@ -134,6 +134,7 @@ export class HarTracer {
       promise
     ]) as Promise<void>;
     this._barrierPromises.add(race);
+    race.then(() => this._barrierPromises.delete(race));
   }
 
   private _onAPIRequest(event: APIRequestEvent) {
diff --git a/tests/har.spec.ts b/tests/har.spec.ts
index ca46351f80..f57c12598f 100644
--- a/tests/har.spec.ts
+++ b/tests/har.spec.ts
@@ -674,3 +674,24 @@ it('should include API request', async ({ contextFactory, server }, testInfo) =>
   expect(entry.response.content.text).toBe(responseBody.toString('base64'));
 });
 
+it('should not hang on resources served from cache', async ({ contextFactory, server, browserName }, testInfo) => {
+  it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/11435' });
+  server.setRoute('/one-style.css', (req, res) => {
+    res.writeHead(200, {
+      'Content-Type': 'text/css',
+      'Cache-Control': 'public, max-age=10031518'
+    });
+    res.end(`body { background: red }`);
+  });
+  const { page, getLog } = await pageWithHar(contextFactory, testInfo);
+  await page.goto(server.PREFIX + '/har.html');
+  await page.goto(server.PREFIX + '/har.html');
+  const log = await getLog();
+  const entries = log.entries.filter(e => e.request.url.endsWith('one-style.css'));
+  // In firefox no request events are fired for cached resources.
+  if (browserName === 'firefox')
+    expect(entries.length).toBe(1);
+  else
+    expect(entries.length).toBe(2);
+});
+