Merge branch 'canary' into darksky/fix-ci

packages/frontend/apps/electron/src/main/windows-manager/tab-views.ts

@@ -391,21 +391,77 @@ export class WebContentViewsManager {
 
     this.closedWorkbenches.push(targetWorkbench);
 
-    setTimeout(() => {
+    globalThis.setTimeout(() => {
       const view = this.tabViewsMap.get(id);
       this.tabViewsMap.delete(id);
 
-      if (this.mainWindow && view) {
-        this.mainWindow.contentView.removeChildView(view);
-        view?.webContents.close({
-          waitForBeforeUnload: true,
-        });
+      if (!view) {
+        return;
       }
+
+      void this.disposeTabView(id, view);
     }, 500); // delay a bit to get rid of the flicker
 
     onTabClose(id);
   };
 
+  private readonly disposeTabView = async (id: string, view: WebContentsView) => {
+    const waitForDestroyed = () =>
+      new Promise<boolean>(resolve => {
+        if (view.webContents.isDestroyed()) {
+          resolve(true);
+          return;
+        }
+
+        const timeout = globalThis.setTimeout(() => {
+          resolve(false);
+        }, 1_000);
+
+        view.webContents.once('destroyed', () => {
+          globalThis.clearTimeout(timeout);
+          resolve(true);
+        });
+      });
+
+    if (this.mainWindow?.contentView.children.includes(view)) {
+      this.mainWindow.contentView.removeChildView(view);
+    }
+
+    if (view.webContents.isDestroyed()) {
+      return;
+    }
+
+    try {
+      view.webContents.close({
+        waitForBeforeUnload: true,
+      });
+    } catch {
+      return;
+    }
+
+    if (await waitForDestroyed()) {
+      return;
+    }
+
+    view.webContents.forcefullyCrashRenderer();
+
+    try {
+      view.webContents.close({
+        waitForBeforeUnload: false,
+      });
+    } catch {
+      return;
+    }
+
+    if (!view.webContents.isDestroyed() && !(await waitForDestroyed())) {
+      logger.warn('tab webContents is still alive after force close', {
+        id,
+        webContentsId: view.webContents.id,
+        url: view.webContents.getURL(),
+      });
+    }
+  };
+
   undoCloseTab = async () => {
     if (this.closedWorkbenches.length === 0) {
       return;

tests/affine-desktop/e2e/tabs.spec.ts

@@ -42,6 +42,7 @@ test('can switch & close tab by clicking', async ({ page }) => {
 
   // the first tab should be active
   await expectActiveTab(page, 0);
+  await expectTabCount(page, 1);
 });
 
 test('Collapse Sidebar', async ({ page }) => {

tests/affine-local/e2e/blocksuite/paragraph.spec.ts

@@ -14,7 +14,7 @@ import {
   waitForEditorLoad,
 } from '@affine-test/kit/utils/page-logic';
 import type { ParagraphBlockComponent } from '@blocksuite/affine-block-paragraph';
-import { expect } from '@playwright/test';
+import { expect, type Locator } from '@playwright/test';
 
 test.beforeEach(async ({ page }) => {
   await openHomePage(page);
@@ -22,6 +22,44 @@ test.beforeEach(async ({ page }) => {
   await waitForEditorLoad(page);
 });
 
+async function expectParagraphState(
+  paragraphs: Locator,
+  index: number,
+  expectedType: string,
+  expectedText: string
+) {
+  await expect
+    .poll(async () => {
+      return await paragraphs
+        .nth(index)
+        .evaluate(
+          (
+            block: ParagraphBlockComponent,
+            expected: { type: string; text: string }
+          ) =>
+            block.model.props.type === expected.type &&
+            block.model.props.text.toString() === expected.text,
+          {
+            type: expectedType,
+            text: expectedText,
+          }
+        );
+    })
+    .toBeTruthy();
+}
+
+async function expectParagraphVisibility(
+  paragraphs: Locator,
+  index: number,
+  visible: boolean
+) {
+  await expect
+    .poll(async () => {
+      return await paragraphs.nth(index).isVisible();
+    })
+    .toBe(visible);
+}
+
 test('heading icon should be updated after change heading level', async ({
   page,
 }) => {
@@ -320,26 +358,9 @@ test('also move children when dedent collapsed heading', async ({ page }) => {
   await subParagraph.nth(0).click();
   await pressShiftTab(page);
   expect(await subParagraph.count()).toBe(0);
-  expect(
-    await paragraph
-      .nth(1)
-      .evaluate(
-        (block: ParagraphBlockComponent) =>
-          block.model.props.type === 'h1' &&
-          block.model.props.text.toString() === 'bbb'
-      )
-  ).toBeTruthy();
-  expect(
-    await paragraph
-      .nth(2)
-      .evaluate(
-        (block: ParagraphBlockComponent) =>
-          block.model.props.type === 'text' &&
-          block.model.props.text.toString() === 'ccc'
-      )
-  ).toBeTruthy();
-
-  expect(await paragraph.nth(2).isVisible()).toBeFalsy();
+  await expectParagraphState(paragraph, 1, 'h1', 'bbb');
+  await expectParagraphState(paragraph, 2, 'text', 'ccc');
+  await expectParagraphVisibility(paragraph, 2, false);
   await paragraph
     .nth(1)
     .locator('blocksuite-toggle-button .toggle-icon')
@@ -349,7 +370,7 @@ test('also move children when dedent collapsed heading', async ({ page }) => {
         y: 5,
       },
     });
-  expect(await paragraph.nth(2).isVisible()).toBeTruthy();
+  await expectParagraphVisibility(paragraph, 2, true);
 });
 
 test('also move collapsed siblings when indent collapsed heading', async ({
@@ -433,23 +454,19 @@ test('unfold collapsed heading when its other blocks indented to be its sibling'
    */
 
   const paragraph = page.locator('affine-note affine-paragraph');
-  expect(await paragraph.nth(2).isVisible()).toBeTruthy();
-  expect(
-    await paragraph
-      .nth(2)
-      .evaluate(
-        (block: ParagraphBlockComponent) =>
-          block.model.props.type === 'text' &&
-          block.model.props.text.toString() === 'ccc'
-      )
-  ).toBeTruthy();
+  await expectParagraphVisibility(paragraph, 2, true);
+  await expectParagraphState(paragraph, 2, 'text', 'ccc');
   await paragraph.locator('blocksuite-toggle-button .toggle-icon').click();
-  expect(await paragraph.nth(2).isVisible()).toBeFalsy();
+  await expectParagraphVisibility(paragraph, 2, false);
 
   await paragraph.nth(3).click(); // ddd
-  expect(await paragraph.nth(2).isVisible()).toBeFalsy();
-  expect(await paragraph.nth(0).locator('affine-paragraph').count()).toBe(2);
+  await expectParagraphVisibility(paragraph, 2, false);
+  await expect
+    .poll(() => paragraph.nth(0).locator('affine-paragraph').count())
+    .toBe(2);
   await pressTab(page);
-  expect(await paragraph.nth(0).locator('affine-paragraph').count()).toBe(3);
-  expect(await paragraph.nth(2).isVisible()).toBeTruthy();
+  await expect
+    .poll(() => paragraph.nth(0).locator('affine-paragraph').count())
+    .toBe(3);
+  await expectParagraphVisibility(paragraph, 2, true);
 });

tests/blocksuite/e2e/utils/actions/misc.ts

@@ -1229,43 +1229,37 @@ export async function getCurrentThemeCSSPropertyValue(
 }
 
 export async function scrollToTop(page: Page) {
-  await page.mouse.wheel(0, -1000);
-
-  await page.waitForFunction(() => {
-    const scrollContainer = document.querySelector('.affine-page-viewport');
-    if (!scrollContainer) {
-      throw new Error("Can't find scroll container");
-    }
-    return scrollContainer.scrollTop < 10;
+  const scrollContainer = page.locator('.affine-page-viewport');
+  await expect(scrollContainer).toBeVisible();
+  await scrollContainer.evaluate(node => {
+    (node as HTMLElement).scrollTop = 0;
   });
+  await expect
+    .poll(async () => {
+      return await scrollContainer.evaluate(node => {
+        return (node as HTMLElement).scrollTop;
+      });
+    })
+    .toBeLessThan(10);
 }
 
 export async function scrollToBottom(page: Page) {
-  // await page.mouse.wheel(0, 1000);
-
-  await page
-    .locator('.affine-page-viewport')
-    .evaluate(node =>
-      node.scrollTo({ left: 0, top: 1000, behavior: 'smooth' })
-    );
-  // TODO switch to `scrollend`
-  // See https://developer.chrome.com/en/blog/scrollend-a-new-javascript-event/
-  await page.waitForFunction(() => {
-    const scrollContainer = document.querySelector('.affine-page-viewport');
-    if (!scrollContainer) {
-      throw new Error("Can't find scroll container");
-    }
-
-    return (
-      // Wait for scrolled to the bottom
-      // Refer to https://stackoverflow.com/questions/3898130/check-if-a-user-has-scrolled-to-the-bottom-not-just-the-window-but-any-element
-      Math.abs(
-        scrollContainer.scrollHeight -
-          scrollContainer.scrollTop -
-          scrollContainer.clientHeight
-      ) < 10
-    );
+  const scrollContainer = page.locator('.affine-page-viewport');
+  await expect(scrollContainer).toBeVisible();
+  await scrollContainer.evaluate(node => {
+    const viewport = node as HTMLElement;
+    viewport.scrollTop = viewport.scrollHeight;
   });
+  await expect
+    .poll(async () => {
+      return await scrollContainer.evaluate(node => {
+        const viewport = node as HTMLElement;
+        return Math.abs(
+          viewport.scrollHeight - viewport.scrollTop - viewport.clientHeight
+        );
+      });
+    })
+    .toBeLessThan(10);
 }
 
 export async function mockParseDocUrlService(

tests/kit/src/electron.ts

@@ -2,32 +2,63 @@ import crypto from 'node:crypto';
 import { setTimeout } from 'node:timers/promises';
 
 import { Package } from '@affine-tools/utils/workspace';
-import { expect, type Page } from '@playwright/test';
+import type { Page } from '@playwright/test';
 import fs from 'fs-extra';
 import type { ElectronApplication } from 'playwright';
 import { _electron as electron } from 'playwright';
+import treeKill from 'tree-kill';
 
-import { test as base, testResultDir } from './playwright';
+import { test as base } from './playwright';
 import { removeWithRetry } from './utils/utils';
 
 const electronRoot = new Package('@affine/electron').path;
 
+const treeKillAsync = (pid: number, signal: NodeJS.Signals) =>
+  new Promise<void>((resolve, reject) => {
+    treeKill(pid, signal, error => {
+      if (
+        !error ||
+        ('code' in error &&
+          typeof error.code === 'string' &&
+          error.code === 'ESRCH')
+      ) {
+        resolve();
+        return;
+      }
+      reject(error);
+    });
+  });
+
 function generateUUID() {
   return crypto.randomUUID();
 }
 
 type RoutePath = 'setting';
 
+const withTimeoutFallback = async <T>(promise: Promise<T>, fallback: T) => {
+  try {
+    return await Promise.race([promise, setTimeout(1_000).then(() => fallback)]);
+  } catch {
+    return fallback;
+  }
+};
+
 const getPageId = async (page: Page) => {
-  return page.evaluate(() => {
-    return (window.__appInfo as any)?.viewId as string;
-  });
+  return await withTimeoutFallback(
+    page.evaluate(() => {
+      return (window.__appInfo as any)?.viewId as string | undefined;
+    }),
+    undefined
+  );
 };
 
 const isActivePage = async (page: Page) => {
-  return page.evaluate(async () => {
-    return await (window as any).__apis?.ui.isActiveTab();
-  });
+  return await withTimeoutFallback(
+    page.evaluate(async () => {
+      return (await (window as any).__apis?.ui.isActiveTab()) === true;
+    }),
+    false
+  );
 };
 
 const getActivePage = async (pages: Page[]) => {
@@ -39,6 +70,106 @@ const getActivePage = async (pages: Page[]) => {
   return null;
 };
 
+const getShellPage = async (pages: Page[]) => {
+  for (const page of pages) {
+    if ((await getPageId(page)) === 'shell') {
+      return page;
+    }
+  }
+  return null;
+};
+
+const waitForElectronPage = async (
+  electronApp: ElectronApplication,
+  label: string,
+  getPage: (pages: Page[]) => Promise<Page | null>
+) => {
+  const deadline = Date.now() + 10_000;
+
+  while (Date.now() < deadline) {
+    const page = await getPage(electronApp.windows());
+    if (page) {
+      return page;
+    }
+
+    await setTimeout(250);
+  }
+
+  throw new Error(`Timed out waiting for ${label}`);
+};
+
+const cleanupElectronApp = async (electronApp: ElectronApplication) => {
+  const child = electronApp.process();
+  const waitForAppClose = () =>
+    new Promise<void>(resolve => {
+      if (child.exitCode !== null || child.signalCode !== null) {
+        resolve();
+        return;
+      }
+      electronApp.once('close', () => resolve());
+    });
+  const waitForProcessExit = () =>
+    new Promise<void>(resolve => {
+      if (child.exitCode !== null || child.signalCode !== null) {
+        resolve();
+        return;
+      }
+      child.once('exit', () => resolve());
+    });
+
+  const killProcess = () => {
+    try {
+      child.kill();
+    } catch {}
+  };
+
+  const closeWithTimeout = async () => {
+    const closeEvent = waitForAppClose();
+    const controller = new AbortController();
+    const killAfterTimeout = setTimeout(10_000, undefined, {
+      signal: controller.signal,
+    })
+      .then(() => {
+        killProcess();
+      })
+      .catch(error => {
+        if (error instanceof Error && error.name === 'AbortError') {
+          return;
+        }
+        throw error;
+      });
+
+    try {
+      await Promise.all([electronApp.close().catch(() => {}), closeEvent]);
+    } finally {
+      controller.abort();
+      await killAfterTimeout;
+    }
+  };
+
+  if (process.env.CI && process.platform === 'linux') {
+    const pid = child.pid;
+    const closeEvent = waitForAppClose();
+    const processExit = waitForProcessExit();
+
+    await Promise.race([
+      Promise.all([electronApp.close().catch(() => {}), closeEvent, processExit]),
+      setTimeout(2_000),
+    ]).catch(() => {});
+
+    if (pid !== undefined && child.exitCode === null && child.signalCode === null) {
+      await treeKillAsync(pid, 'SIGKILL').catch(() => {});
+    }
+
+    await Promise.race([closeEvent, processExit, setTimeout(5_000)]).catch(
+      () => {}
+    );
+    return;
+  }
+
+  await closeWithTimeout();
+};
+
 export const test = base.extend<{
   electronApp: ElectronApplication;
   shell: Page;
@@ -55,53 +186,27 @@ export const test = base.extend<{
   };
 }>({
   shell: async ({ electronApp }, use) => {
-    await expect.poll(() => electronApp.windows().length > 1).toBeTruthy();
+    const shell = await waitForElectronPage(
+      electronApp,
+      'shell page',
+      getShellPage
+    );
 
-    for (const page of electronApp.windows()) {
-      const viewId = await getPageId(page);
-      if (viewId === 'shell') {
-        await use(page);
-        break;
-      }
-    }
+    await use(shell);
   },
   page: async ({ electronApp }, use) => {
-    await expect
-      .poll(
-        () => {
-          return electronApp.windows().length > 1;
-        },
-        {
-          timeout: 10000,
-        }
-      )
-      .toBeTruthy();
+    const page = await waitForElectronPage(
+      electronApp,
+      'active page',
+      getActivePage
+    );
 
-    await expect
-      .poll(
-        async () => {
-          const page = await getActivePage(electronApp.windows());
-          return !!page;
-        },
-        {
-          timeout: 10000,
-        }
-      )
-      .toBeTruthy();
-
-    const page = await getActivePage(electronApp.windows());
-
-    if (!page) {
-      throw new Error('No active page found');
-    }
-
-    // wait for blocksuite to be loaded
     await page.waitForSelector('v-line');
 
-    await use(page as Page);
+    await use(page);
   },
   views: async ({ electronApp, page }, use) => {
-    void page; // makes sure page is a dependency
+    void page;
     await use({
       getActive: async () => {
         const view = await getActivePage(electronApp.windows());
@@ -111,20 +216,18 @@ export const test = base.extend<{
   },
   // oxlint-disable-next-line no-empty-pattern
   electronApp: async ({}, use) => {
+    const id = generateUUID();
+    const dist = electronRoot.join('dist').value;
+    const clonedDist = electronRoot.join('e2e-dist-' + id).value;
+    let electronApp: ElectronApplication | undefined;
+
     try {
-      // a random id to avoid conflicts between tests
-      const id = generateUUID();
-      const dist = electronRoot.join('dist').value;
-      const clonedDist = electronRoot.join('e2e-dist-' + id).value;
       await fs.copy(dist, clonedDist);
       const packageJson = await fs.readJSON(
         electronRoot.join('package.json').value
       );
-      // overwrite the app name
       packageJson.name = '@affine/electron-test-' + id;
-      // overwrite the path to the main script
       packageJson.main = './main.js';
-      // write to the cloned dist
       await fs.writeJSON(clonedDist + '/package.json', packageJson);
 
       const env: Record<string, string> = {};
@@ -134,41 +237,23 @@ export const test = base.extend<{
         }
       }
       env.DEBUG = 'pw:browser';
-
       env.SKIP_ONBOARDING = '1';
 
-      const electronApp = await electron.launch({
+      electronApp = await electron.launch({
         args: [clonedDist],
         env,
         cwd: clonedDist,
-        recordVideo: {
-          dir: testResultDir,
-        },
         colorScheme: 'light',
       });
 
       await use(electronApp);
-      const cleanup = async () => {
-        const pages = electronApp.windows();
-        for (const page of pages) {
-          if (page.isClosed()) {
-            continue;
-          }
-          await page.close();
-        }
-        await electronApp.close();
+    } finally {
+      if (electronApp) {
+        await cleanupElectronApp(electronApp);
+      }
+      if (await fs.pathExists(clonedDist)) {
         await removeWithRetry(clonedDist);
-      };
-      await Promise.race([
-        // cleanup may stuck and fail the test, but it should be fine.
-        cleanup(),
-        setTimeout(10000).then(() => {
-          // kill the electron app if it is not closed after 10 seconds
-          electronApp.process().kill();
-        }),
-      ]);
-    } catch (error) {
-      console.log(error);
+      }
     }
   },
   appInfo: async ({ electronApp }, use) => {