fix(video): wait for videos when closing persistent context (#4040)

To achieve this, we close all the pages one by one, then wait
for the videos to finish processing, and then close the browser.

browsers.json

@@ -8,7 +8,7 @@
     },
     {
       "name": "firefox",
-      "revision": "1179",
+      "revision": "1180",
       "download": true
     },
     {

src/server/browserContext.ts

@@ -265,15 +265,10 @@ export abstract class BrowserContext extends EventEmitter {
   }
 
   async close() {
-    if (this._isPersistentContext) {
-      // Default context is only created in 'persistent' mode and closing it should close
-      // the browser.
-      await this._browser.close();
-      return;
-    }
     if (this._closedStatus === 'open') {
       this._closedStatus = 'closing';
-      await this._doClose();
+
+      // Collect videos/downloads that we will await.
       const promises: Promise<any>[] = [];
       for (const download of this._downloads)
         promises.push(download.delete());
@@ -281,7 +276,24 @@ export abstract class BrowserContext extends EventEmitter {
         if (video._context === this)
           promises.push(video._finishedPromise);
       }
+
+      if (this._isPersistentContext) {
+        // Close all the pages instead of the context,
+        // because we cannot close the default context.
+        await Promise.all(this.pages().map(page => page.close()));
+      } else {
+        // Close the context.
+        await this._doClose();
+      }
+
+      // Wait for the videos/downloads to finish.
       await Promise.all(promises);
+
+      // Persistent context should also close the browser.
+      if (this._isPersistentContext)
+        await this._browser.close();
+
+      // Bookkeeping.
       for (const listener of contextListeners)
         await listener.onContextDestroyed(this);
       this._didCloseInternal();

test/downloads-path.spec.ts

@@ -83,7 +83,6 @@ it('should delete downloads when context closes', async ({downloadsBrowser, serv
   expect(fs.existsSync(path)).toBeTruthy();
   await page.close();
   expect(fs.existsSync(path)).toBeFalsy();
-
 });
 
 it('should report downloads in downloadsPath folder', async ({downloadsBrowser, testOutputPath, server}) => {
@@ -98,7 +97,7 @@ it('should report downloads in downloadsPath folder', async ({downloadsBrowser,
   await page.close();
 });
 
-it('should accept downloads', async ({persistentDownloadsContext, testOutputPath, server})  => {
+it('should accept downloads in persistent context', async ({persistentDownloadsContext, testOutputPath, server})  => {
   const page = persistentDownloadsContext.pages()[0];
   const [ download ] = await Promise.all([
     page.waitForEvent('download'),
@@ -110,13 +109,14 @@ it('should accept downloads', async ({persistentDownloadsContext, testOutputPath
   expect(path.startsWith(testOutputPath(''))).toBeTruthy();
 });
 
-it('should not delete downloads when the context closes', async ({persistentDownloadsContext}) => {
+it('should delete downloads when persistent context closes', async ({persistentDownloadsContext}) => {
   const page = persistentDownloadsContext.pages()[0];
   const [ download ] = await Promise.all([
     page.waitForEvent('download'),
     page.click('a')
   ]);
   const path = await download.path();
-  await persistentDownloadsContext.close();
   expect(fs.existsSync(path)).toBeTruthy();
+  await persistentDownloadsContext.close();
+  expect(fs.existsSync(path)).toBeFalsy();
 });

test/screencast.spec.ts

@@ -341,9 +341,7 @@ describe('screencast', suite => {
     expect(await videoPlayer.videoHeight).toBe(720);
   });
 
-  it('should capture static page in persistent context', test => {
+  it('should capture static page in persistent context', async ({launchPersistent, testOutputPath}) => {
-    test.fixme('We do not wait for the video finish when closing persistent context.');
-  }, async ({launchPersistent, testOutputPath}) => {
     const videosPath = testOutputPath('');
     const size = { width: 320, height: 240 };
     const { context, page } = await launchPersistent({