From 8bc26a2b4471473d2af8c6677f26f8ebb6b2a1cb Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Tue, 6 Jun 2023 20:38:36 +0200
Subject: [PATCH] chore: wait for downloads getting removed on context.close()
 (#23500)

Before there was a race, that we ran into this code:


https://github.com/microsoft/playwright/blob/9cd49d5dd5b4074fdfa66b463b0a3379e73bcd69/packages/playwright-core/src/server/browserContext.ts#L236-L237

and then into this code:


https://github.com/microsoft/playwright/blob/9cd49d5dd5b4074fdfa66b463b0a3379e73bcd69/packages/playwright-core/src/server/browserContext.ts#L429-L431

which had the side effect, that the first call did not wait. Then
immediately clears the downloads Set and then the second call is a NOOP.
This ends up that the the removal of the downloads can happen after the
context is closed, hence the test is flaky.

Relates to https://github.com/microsoft/playwright/pull/6151 where it
got introduced. So something for @yury-s.

Fixes https://github.com/microsoft/playwright/issues/22525
---
 packages/playwright-core/src/server/browserContext.ts | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts
index e0515d3f3e..5469028964 100644
--- a/packages/playwright-core/src/server/browserContext.ts
+++ b/packages/playwright-core/src/server/browserContext.ts
@@ -232,9 +232,12 @@ export abstract class BrowserContext extends SdkObject {
       // at the same time.
       return;
     }
+    const gotClosedGracefully = this._closedStatus === 'closing';
     this._closedStatus = 'closed';
-    this._deleteAllDownloads();
-    this._downloads.clear();
+    if (!gotClosedGracefully) {
+      this._deleteAllDownloads();
+      this._downloads.clear();
+    }
     this.tracing.dispose().catch(() => {});
     if (this._isPersistentContext)
       this.onClosePersistent();