Fix startup document restore order and focus

Author: reycn

electron/document-session.mjs

@@ -41,8 +41,7 @@ export async function createDocumentSession(path) {
     return writes;
   }
   return {
-    restore: () =>
-      structuredClone(saved.documents.length ? saved.documents : saved.last ? [saved.last] : []),
+    restore: () => structuredClone(saved.documents),
     open(key, value) {
       const document = entry(value);
       if (!document) throw Error('Invalid session document');

electron/main.mjs

@@ -628,31 +628,34 @@ else {
     return focusDocumentWindow(target);
   }
   const openingDocuments = new Map();
-  async function openDocumentWindow(document, preferredWindow, restoreView) {
+  async function openDocumentWindow(document, preferredWindow, restoreView, activate = true) {
     const started = performance.now();
     try {
-      return await deliverDocumentWindow(document, preferredWindow, restoreView);
+      return await deliverDocumentWindow(document, preferredWindow, restoreView, activate);
     } finally {
       developerOperationMetrics.fileOpen(performance.now() - started);
     }
   }
-  async function deliverDocumentWindow(document, preferredWindow, restoreView) {
+  async function deliverDocumentWindow(document, preferredWindow, restoreView, activate) {
     const identity = typeof document === 'string' ? documentIdentity(document) : null;
     const existing = findDocumentWindow(identity);
-    if (existing) return focusDocumentWindow(existing);
-    if (identity && openingDocuments.has(identity))
-      return focusDocumentWindow(await openingDocuments.get(identity));
+    if (existing) return activate ? focusDocumentWindow(existing) : existing;
+    if (identity && openingDocuments.has(identity)) {
+      const target = await openingDocuments.get(identity);
+      return activate ? focusDocumentWindow(target) : target;
+    }
     const operation = (async () => {
       const candidates = [preferredWindow, focusedWindow(), ...windows.keys()];
       const target = candidates.find(isBlankStartPage);
-      if (!target) return createWindow(document, restoreView);
+      if (!target) return createWindow(document, restoreView, null, 'general', activate);
       const state = windows.get(target);
       state.documentIdentity = identity;
+      state.restoreView = restoreView;
       state.documents.push(document);
       state.unkeyedAnnotationSource = annotationSourceForDocument(document);
       state.performance.hasDocument = true;
       target.webContents.send('documents:available');
-      return focusDocumentWindow(target);
+      return activate ? focusDocumentWindow(target) : target;
     })();
     if (identity) openingDocuments.set(identity, operation);
     try {
@@ -666,6 +669,7 @@ else {
     restoreView,
     settingsOwner = null,
     settingsSection = 'general',
+    activate = true,
   ) {
     let window;
     const backend = settingsOwner
@@ -912,7 +916,7 @@ else {
     const showLoadedWindow = () => {
       hideNativeMenuBar(window);
       if (!backgroundRenderSmoke && !window.isVisible()) {
-        if (process.argv.includes('--background')) window.showInactive();
+        if (!activate || process.argv.includes('--background')) window.showInactive();
         else window.show();
       }
       updatePerformanceSampling(window);
@@ -2337,22 +2341,32 @@ else {
         trustedWindow(event);
         await createWindow();
       });
+      // Drain external launch requests before restoring the previous session. Requests
+      // arriving while a backend starts also take priority over the next restore.
+      let externalLaunch = false;
+      const openLaunchFiles = async () => {
+        if (ciLaunchCheck || !pendingFiles.length) return;
+        externalLaunch = true;
+        await deliverPendingFiles();
+      };
+      await openLaunchFiles();
       if (!smoke && !ciLaunchCheck && preferences.load().restoreDocuments) {
         for (const document of documentSession.restore()) {
+          await openLaunchFiles();
           try {
             await validateSystemPDF(document.path);
           } catch {
             continue;
           }
-          if (!pendingFiles.includes(document.path))
-            await openDocumentWindow(document.path, null, document.view);
+          await openLaunchFiles();
+          const firstRestore = windows.size === 0;
+          const restored = await openDocumentWindow(document.path, null, document.view, false);
+          await openLaunchFiles();
+          if (firstRestore && !externalLaunch) focusDocumentWindow(restored);
         }
       }
-      window =
-        [...windows.keys()][0] ||
-        (!ciLaunchCheck && pendingFiles.length
-          ? await openDocumentWindow(pendingFiles.shift())
-          : await createWindow());
+      await openLaunchFiles();
+      window = [...windows.keys()][0] || (await createWindow());
       backend = windows.get(window).backend;
       documentsReady = true;
       if (!ciLaunchCheck) await deliverPendingFiles();

electron/session-smoke.mjs

@@ -34,10 +34,20 @@ export async function verifySession(first, windows, createWindow) {
       sidebar: false,
       showTranslations: false,
     };
-    const second = await createWindow(a, view),
-      third = await createWindow(b);
+    const second = await createWindow(a, view);
     await wait(second, 'window.previewReady');
+    second.show();
+    second.focus();
+    for (let i = 0; i < 100 && !second.isFocused(); i++)
+      await new Promise((resolve) => setTimeout(resolve, 50));
+    assert.equal(second.isFocused(), true);
+    const third = await createWindow(b, undefined, null, 'general', false);
     await wait(third, 'window.previewReady');
+    for (let i = 0; i < 100 && !third.isVisible(); i++)
+      await new Promise((resolve) => setTimeout(resolve, 50));
+    assert.equal(third.isVisible(), true);
+    assert.equal(third.isFocused(), false);
+    assert.equal(second.isFocused(), true);
     await second.webContents.executeJavaScript('window.previewSaveReadingView()');
     await third.webContents.executeJavaScript('window.previewSaveReadingView()');
     const restored = (await createDocumentSession(store)).restore();
@@ -51,9 +61,7 @@ export async function verifySession(first, windows, createWindow) {
     );
     third.webContents.send('reader:action', 'close-document');
     await wait(third, 'window.previewRenderDiagnostics().totalPages===0');
-    const fallback = (await createDocumentSession(store)).restore();
-    assert.equal(fallback.length, 1);
-    assert.equal(fallback[0].path, b);
+    assert.deepEqual((await createDocumentSession(store)).restore(), []);
     await first.webContents.executeJavaScript(
       'window.previewPreferences.save({restoreDocuments:false})',
     );
@@ -67,8 +75,9 @@ export async function verifySession(first, windows, createWindow) {
       JSON.stringify({
         passed: true,
         multipleDocuments: true,
+        backgroundRestorePreservesFocus: true,
         restoredPosition: true,
-        lastClosedFallback: true,
+        allDocumentsClosed: true,
         sharedOptOut: true,
       }),
     );

server/document-session.test.mjs

@@ -16,7 +16,7 @@ const view = {
   showTranslations: true,
 };
 
-test('session persists multiple documents and positions; closing all restores only the last closed document', async () => {
+test('session persists multiple documents and positions; closing all restores nothing', async () => {
   const dir = await mkdtemp(join(tmpdir(), 'reader-session-'));
   try {
     const path = join(dir, 'session.json'),
@@ -36,9 +36,26 @@ test('session persists multiple documents and positions; closing all restores on
     assert.deepEqual((await createDocumentSession(path)).restore(), [{ path: '/tmp/a.pdf', view }]);
     await session.close(1);
     await session.close(1);
-    assert.deepEqual((await createDocumentSession(path)).restore(), [{ path: '/tmp/a.pdf', view }]);
+    assert.deepEqual((await createDocumentSession(path)).restore(), []);
     assert.equal(JSON.parse(await readFile(path)).documents.length, 0);
   } finally {
     await rm(dir, { recursive: true, force: true });
   }
 });
+
+test('empty documents do not restore a legacy last closed document', async () => {
+  const dir = await mkdtemp(join(tmpdir(), 'reader-session-'));
+  try {
+    const path = join(dir, 'session.json');
+    await writeFile(
+      path,
+      JSON.stringify({
+        documents: [],
+        last: { path: '/tmp/legacy-closed.pdf' },
+      }),
+    );
+    assert.deepEqual((await createDocumentSession(path)).restore(), []);
+  } finally {
+    await rm(dir, { recursive: true, force: true });
+  }
+});