Skip to content

Commit

Permalink
fix: BrowserWindow lifecycle (hard to reproduce, so safeguards everyw…
Browse files Browse the repository at this point in the history
…here) Follows PR #2640
  • Loading branch information
danielweck committed Nov 13, 2024
1 parent 94ead59 commit 79a62f3
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 40 deletions.
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@
"css-hot-loader": "^1.4.4",
"css-loader": "^7.1.2",
"devtron": "^1.4.0",
"electron": "^33.1.0",
"electron": "^33.2.0",
"electron-builder": "^25.1.8",
"electron-devtools-installer": "^3.2.0",
"eslint": "^8.57.1",
Expand Down
2 changes: 1 addition & 1 deletion src/main/cli/lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export function lockInstance() {
// https://github.com/edrlab/thorium-reader/pull/1573#issuecomment-1003042325
try {
const libraryAppWindow = getLibraryWindowFromDi();
if (libraryAppWindow) {
if (libraryAppWindow && !libraryAppWindow.isDestroyed() && !libraryAppWindow.webContents.isDestroyed()) {
if (libraryAppWindow.isMinimized()) {
libraryAppWindow.restore();
}
Expand Down
5 changes: 4 additions & 1 deletion src/main/redux/middleware/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ export const reduxSyncMiddleware: Middleware
if (libId) {
try {
const libWin = getLibraryWindowFromDi();
browserWin.set(libId, libWin);
if (libWin && !libWin.isDestroyed() && !libWin.webContents.isDestroyed()) {
browserWin.set(libId, libWin);
}
} catch (_err) {
// ignore
// library window may be not initialized in first
Expand All @@ -126,6 +128,7 @@ export const reduxSyncMiddleware: Middleware
if (readers[key]) {
try {
const readerWin = getReaderWindowFromDi(readers[key].identifier);
if (!readerWin.isDestroyed() && !readerWin.webContents.isDestroyed())
browserWin.set(readers[key].identifier, readerWin);
} catch (_err) {
// ignore
Expand Down
3 changes: 2 additions & 1 deletion src/main/redux/sagas/annotation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ function* importAnnotationSet(action: annotationActions.importAnnotationSet.TAct
const currentTimestamp = (new Date()).getTime();

const win = winId ? getReaderWindowFromDi(winId) : getLibraryWindowFromDi();
if (!win) {

if (!win || win.isDestroyed() || win.webContents.isDestroyed()) {
debug("ERROR!! No Browser window !!! exit");
return ;
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/redux/sagas/api/publication/export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import { diMainGet, getLibraryWindowFromDi } from "readium-desktop/main/di";
export function* exportPublication(publicationView: PublicationView) {

const libraryAppWindow = yield* callTyped(() => getLibraryWindowFromDi());
if (!libraryAppWindow || libraryAppWindow.isDestroyed() || libraryAppWindow.webContents.isDestroyed()) {
return;
}

const publicationStorage = diMainGet("publication-storage");

const defaultFilename = publicationStorage.getPublicationFilename(publicationView);
Expand Down
11 changes: 8 additions & 3 deletions src/main/redux/sagas/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function* init() {
const browserWindows = [];
try {
const libraryWin = getLibraryWindowFromDi();
if (libraryWin) {
if (libraryWin && !libraryWin.isDestroyed() && !libraryWin.webContents.isDestroyed()) {
browserWindows.push(libraryWin);
}
} catch (e) {
Expand All @@ -80,7 +80,11 @@ export function* init() {
try {
const readerWins = getAllReaderWindowFromDi();
if (readerWins?.length) {
browserWindows.push(...readerWins);
for (const readerWin of readerWins) {
if (readerWin && !readerWin.isDestroyed() && !readerWin.webContents.isDestroyed()) {
browserWindows.push(readerWin);
}
}
}
} catch (e) {
debug("getAllReaderWindowFromDi ERROR?", e);
Expand Down Expand Up @@ -331,7 +335,6 @@ export function exit() {
shouldExit = true;

const libraryWin = getLibraryWindowFromDi();

if (process.platform === "darwin") {
if (libraryWin.isDestroyed()) {
if (closeProcessLock.isLock) {
Expand All @@ -345,7 +348,9 @@ export function exit() {
}
}

if (libraryWin && !libraryWin.isDestroyed() && !libraryWin.webContents.isDestroyed()) {
libraryWin.close();
}
};

yield takeSpawnEveryChannel(
Expand Down
2 changes: 1 addition & 1 deletion src/main/redux/sagas/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ function opdsAuthDocConverter(doc: OPDSAuthenticationDoc, baseUrl: string): IOPD
function createOpdsAuthenticationModalWin(url: string): BrowserWindow | undefined {

const libWin = tryCatchSync(() => getLibraryWindowFromDi(), filename_);
if (!libWin) {
if (!libWin || libWin.isDestroyed() || libWin.webContents.isDestroyed()) {
debug("no lib win !!");
return undefined;
}
Expand Down
17 changes: 10 additions & 7 deletions src/main/redux/sagas/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ function* readerFullscreenRequest(action: readerActions.fullScreenRequest.TActio
if (sender.identifier && sender.type === SenderType.Renderer) {

const readerWin = yield* callTyped(() => getReaderWindowFromDi(sender.identifier));
if (readerWin) {

if (readerWin && !readerWin.isDestroyed() && !readerWin.webContents.isDestroyed()) {
readerWin.setFullScreen(action.payload.full);
}
}
Expand All @@ -54,7 +55,7 @@ function* readerFullscreenRequest(action: readerActions.fullScreenRequest.TActio
function* readerDetachRequest(action: readerActions.detachModeRequest.TAction) {

const libWin = yield* callTyped(() => getLibraryWindowFromDi());
if (libWin && !libWin.isDestroyed()) {
if (libWin && !libWin.isDestroyed() && !libWin.webContents.isDestroyed()) {

// try-catch to do not trigger an error message when the winbound is not handle by the os
let libBound: Electron.Rectangle;
Expand All @@ -80,8 +81,7 @@ function* readerDetachRequest(action: readerActions.detachModeRequest.TAction) {
if (readerWinId && action.sender?.type === SenderType.Renderer) {

const readerWin = getReaderWindowFromDi(readerWinId);

if (readerWin) {
if (readerWin && !readerWin.isDestroyed() && !readerWin.webContents.isDestroyed()) {

// this should never occur, but let's do it for certainty
if (readerWin.isMinimized()) {
Expand Down Expand Up @@ -230,7 +230,10 @@ function* readerOpenRequest(action: readerActions.openRequest.TAction) {
const mode = yield* selectTyped((state: RootState) => state.mode);
if (mode === ReaderMode.Attached) {
try {
getLibraryWindowFromDi().hide();
const libWin = getLibraryWindowFromDi();
if (libWin && !libWin.isDestroyed() && !libWin.webContents.isDestroyed()) {
libWin.hide();
}
} catch (_err) {
debug("library can't be loaded from di");
}
Expand Down Expand Up @@ -262,7 +265,7 @@ function* readerCLoseRequestFromIdentifier(action: readerActions.closeRequest.TA
yield call(readerCloseRequest, action.sender.identifier);

const libWin = yield* callTyped(() => getLibraryWindowFromDi());
if (libWin && !libWin.isDestroyed()) {
if (libWin && !libWin.isDestroyed() && !libWin.webContents.isDestroyed()) {

const winBound = yield* selectTyped(
(state: RootState) => state.win.session.library.windowBound,
Expand Down Expand Up @@ -313,7 +316,7 @@ function* readerCloseRequest(identifier?: string) {
}

const readerWindow = getReaderWindowFromDi(identifier);
if (readerWindow) {
if (readerWindow && !readerWindow.isDestroyed() && !readerWindow.webContents.isDestroyed()) {
readerWindow.close();
}

Expand Down
23 changes: 12 additions & 11 deletions src/main/redux/sagas/win/library.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function* appActivate() {

const libWin = yield* callTyped(() => getLibraryWindowFromDi());

if (!libWin?.isDestroyed()) {
if (libWin && !libWin.isDestroyed() && !libWin.webContents.isDestroyed()) {

if (libWin.isMinimized()) {
libWin.restore();
Expand All @@ -65,11 +65,12 @@ export function* appActivate() {
const readers = yield* selectTyped((state: RootState) => state.win.session.reader);
const readersArray = ObjectKeys(readers);
const readerWin = getReaderWindowFromDi(readersArray[0]);

if (readerWin.isMinimized()) {
readerWin.restore();
if (readerWin && !readerWin.isDestroyed() && !readerWin.webContents.isDestroyed()) {
if (readerWin.isMinimized()) {
readerWin.restore();
}
readerWin.show();
}
readerWin.show();
}

return ;
Expand Down Expand Up @@ -189,7 +190,7 @@ function* winClose(_action: winActions.library.closed.TAction) {

debug("library -> winClose");

const library = getLibraryWindowFromDi();
const libraryWin = getLibraryWindowFromDi();
let sessionSaving = false; // window.close() // not saved session by default

{
Expand Down Expand Up @@ -242,7 +243,7 @@ function* winClose(_action: winActions.library.closed.TAction) {
}
try {
const readerWin = yield* callTyped(() => getReaderWindowFromDi(reader.identifier));

if (readerWin && !readerWin.isDestroyed() && !readerWin.webContents.isDestroyed())
if (sessionSaving) {
// force quit the reader windows to keep session in next startup
debug("destroy reader", index);
Expand All @@ -264,9 +265,8 @@ function* winClose(_action: winActions.library.closed.TAction) {
}

if (sessionSaving) {

// closed the library and thorium
library.destroy();
if (libraryWin && !libraryWin.isDestroyed() && !libraryWin.webContents.isDestroyed())
libraryWin.destroy();
} else {

yield spawn(function* () {
Expand All @@ -280,7 +280,8 @@ function* winClose(_action: winActions.library.closed.TAction) {

} while (readersArray.length);

library.destroy();
if (libraryWin && !libraryWin.isDestroyed() && !libraryWin.webContents.isDestroyed())
libraryWin.destroy();
});

}
Expand Down
20 changes: 11 additions & 9 deletions src/main/redux/sagas/win/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function* winClose(action: winActions.reader.closed.TAction) {
const readersArray = ObjectValues(readers);

try {
const libraryWindow = yield* callTyped(() => getLibraryWindowFromDi());
const libraryWin = yield* callTyped(() => getLibraryWindowFromDi());

debug("Nb of readers:", readersArray.length);
debug("readers: ", readersArray);
Expand All @@ -134,27 +134,29 @@ function* winClose(action: winActions.reader.closed.TAction) {

} else {
const readerWin = yield* callTyped(() => getReaderWindowFromDi(identifier));
if (readerWin) {
if (readerWin && !readerWin.isDestroyed() && !readerWin.webContents.isDestroyed()) {
try {
const winBound = readerWin.getBounds();
debug("_______3 readerWin.getBounds()", winBound);
normalizeRectangle(winBound);
libraryWindow.setBounds(winBound);

if (libraryWin && !libraryWin.isDestroyed() && !libraryWin.webContents.isDestroyed())
libraryWin.setBounds(winBound);
} catch (e) {
debug("error libraryWindow.setBounds(readerWin.getBounds())", e);
}
}
}
}

if (libraryWindow) {
if (libraryWindow.isMinimized()) {
libraryWindow.restore();
} else if (!libraryWindow.isVisible()) {
libraryWindow.close();
if (libraryWin && !libraryWin.isDestroyed() && !libraryWin.webContents.isDestroyed()) {
if (libraryWin.isMinimized()) {
libraryWin.restore();
} else if (!libraryWin.isVisible()) {
libraryWin.close();
return;
}
libraryWindow.show(); // focuses as well
libraryWin.show(); // focuses as well
}

} catch (_err) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/tools/showLibrary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { getAppActivateEventChannel } from "../redux/sagas/getEventChannel";
export const showLibrary = () => {

const library = getLibraryWindowFromDi();
if (library.isDestroyed()) {
if (!library || library.isDestroyed() || library.webContents.isDestroyed()) {

const appActivateChannel = getAppActivateEventChannel();
appActivateChannel.put(true);
Expand Down

0 comments on commit 79a62f3

Please sign in to comment.