diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index f7c3d888..adee8a61 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -9,6 +9,8 @@ #include "CrossPointState.h" #include "EpubReaderChapterSelectionActivity.h" #include "EpubReaderPercentSelectionActivity.h" +#include "KOReaderCredentialStore.h" +#include "KOReaderSyncActivity.h" #include "MappedInputManager.h" #include "RecentBooksStore.h" #include "components/UITheme.h" @@ -140,6 +142,45 @@ void EpubReaderActivity::loop() { // Pass input responsibility to sub activity if exists if (subActivity) { subActivity->loop(); + // Deferred exit: process after subActivity->loop() returns to avoid use-after-free + if (pendingSubactivityExit) { + pendingSubactivityExit = false; + exitActivity(); + updateRequired = true; + skipNextButtonCheck = true; // Skip button processing to ignore stale events + } + // Deferred go home: process after subActivity->loop() returns to avoid race condition + if (pendingGoHome) { + pendingGoHome = false; + exitActivity(); + if (onGoHome) { + onGoHome(); + } + return; // Don't access 'this' after callback + } + return; + } + + // Handle pending go home when no subactivity (e.g., from long press back) + if (pendingGoHome) { + pendingGoHome = false; + if (onGoHome) { + onGoHome(); + } + return; // Don't access 'this' after callback + } + + // Skip button processing after returning from subactivity + // This prevents stale button release events from triggering actions + // We wait until: (1) all relevant buttons are released, AND (2) wasReleased events have been cleared + if (skipNextButtonCheck) { + const bool confirmCleared = !mappedInput.isPressed(MappedInputManager::Button::Confirm) && + !mappedInput.wasReleased(MappedInputManager::Button::Confirm); + const bool backCleared = !mappedInput.isPressed(MappedInputManager::Button::Back) && + !mappedInput.wasReleased(MappedInputManager::Button::Back); + if (confirmCleared && backCleared) { + skipNextButtonCheck = false; + } return; } @@ -387,11 +428,8 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction break; } case EpubReaderMenuActivity::MenuAction::GO_HOME: { - // 2. Trigger the reader's "Go Home" callback - if (onGoHome) { - onGoHome(); - } - + // Defer go home to avoid race condition with display task + pendingGoHome = true; break; } case EpubReaderMenuActivity::MenuAction::DELETE_CACHE: { @@ -412,10 +450,34 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction saveProgress(backupSpine, backupPage, backupPageCount); } - exitActivity(); - updateRequired = true; xSemaphoreGive(renderingMutex); - if (onGoHome) onGoHome(); + // Defer go home to avoid race condition with display task + pendingGoHome = true; + break; + } + case EpubReaderMenuActivity::MenuAction::SYNC: { + if (KOREADER_STORE.hasCredentials()) { + xSemaphoreTake(renderingMutex, portMAX_DELAY); + const int currentPage = section ? section->currentPage : 0; + const int totalPages = section ? section->pageCount : 0; + exitActivity(); + enterNewActivity(new KOReaderSyncActivity( + renderer, mappedInput, epub, epub->getPath(), currentSpineIndex, currentPage, totalPages, + [this]() { + // On cancel - defer exit to avoid use-after-free + pendingSubactivityExit = true; + }, + [this](int newSpineIndex, int newPage) { + // On sync complete - update position and defer exit + if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) { + currentSpineIndex = newSpineIndex; + nextPageNumber = newPage; + section.reset(); + } + pendingSubactivityExit = true; + })); + xSemaphoreGive(renderingMutex); + } break; } } diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index c1738871..3ec1196a 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -24,6 +24,9 @@ class EpubReaderActivity final : public ActivityWithSubactivity { // Normalized 0.0-1.0 progress within the target spine item, computed from book percentage. float pendingSpineProgress = 0.0f; bool updateRequired = false; + bool pendingSubactivityExit = false; // Defer subactivity exit to avoid use-after-free + bool pendingGoHome = false; // Defer go home to avoid race condition with display task + bool skipNextButtonCheck = false; // Skip button processing for one frame after subactivity exit const std::function onGoBack; const std::function onGoHome; diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp index 762580d9..ae3a032c 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp @@ -2,10 +2,6 @@ #include -#include - -#include "KOReaderCredentialStore.h" -#include "KOReaderSyncActivity.h" #include "MappedInputManager.h" #include "components/UITheme.h" #include "fontIds.h" @@ -15,25 +11,7 @@ namespace { constexpr int SKIP_PAGE_MS = 700; } // namespace -bool EpubReaderChapterSelectionActivity::hasSyncOption() const { return KOREADER_STORE.hasCredentials(); } - -int EpubReaderChapterSelectionActivity::getTotalItems() const { - // Add 2 for sync options (top and bottom) if credentials are configured - const int syncCount = hasSyncOption() ? 2 : 0; - return epub->getTocItemsCount() + syncCount; -} - -bool EpubReaderChapterSelectionActivity::isSyncItem(int index) const { - if (!hasSyncOption()) return false; - // First item and last item are sync options - return index == 0 || index == getTotalItems() - 1; -} - -int EpubReaderChapterSelectionActivity::tocIndexFromItemIndex(int itemIndex) const { - // Account for the sync option at the top - const int offset = hasSyncOption() ? 1 : 0; - return itemIndex - offset; -} +int EpubReaderChapterSelectionActivity::getTotalItems() const { return epub->getTocItemsCount(); } int EpubReaderChapterSelectionActivity::getPageItems() const { // Layout constants used in renderScreen @@ -65,13 +43,10 @@ void EpubReaderChapterSelectionActivity::onEnter() { renderingMutex = xSemaphoreCreateMutex(); - // Account for sync option offset when finding current TOC index - const int syncOffset = hasSyncOption() ? 1 : 0; selectorIndex = epub->getTocIndexForSpineIndex(currentSpineIndex); if (selectorIndex == -1) { selectorIndex = 0; } - selectorIndex += syncOffset; // Offset for top sync option // Trigger first update updateRequired = true; @@ -96,24 +71,6 @@ void EpubReaderChapterSelectionActivity::onExit() { renderingMutex = nullptr; } -void EpubReaderChapterSelectionActivity::launchSyncActivity() { - xSemaphoreTake(renderingMutex, portMAX_DELAY); - exitActivity(); - enterNewActivity(new KOReaderSyncActivity( - renderer, mappedInput, epub, epubPath, currentSpineIndex, currentPage, totalPagesInSpine, - [this]() { - // On cancel - exitActivity(); - updateRequired = true; - }, - [this](int newSpineIndex, int newPage) { - // On sync complete - exitActivity(); - onSyncPosition(newSpineIndex, newPage); - })); - xSemaphoreGive(renderingMutex); -} - void EpubReaderChapterSelectionActivity::loop() { if (subActivity) { subActivity->loop(); @@ -130,15 +87,7 @@ void EpubReaderChapterSelectionActivity::loop() { const int totalItems = getTotalItems(); if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { - // Check if sync option is selected (first or last item) - if (isSyncItem(selectorIndex)) { - launchSyncActivity(); - return; - } - - // Get TOC index (account for top sync offset) - const int tocIndex = tocIndexFromItemIndex(selectorIndex); - const auto newSpineIndex = epub->getSpineIndexForTocIndex(tocIndex); + const auto newSpineIndex = epub->getSpineIndexForTocIndex(selectorIndex); if (newSpineIndex == -1) { onGoBack(); } else { @@ -209,20 +158,14 @@ void EpubReaderChapterSelectionActivity::renderScreen() { const int displayY = 60 + contentY + i * 30; const bool isSelected = (itemIndex == selectorIndex); - if (isSyncItem(itemIndex)) { - // Sync option uses a fixed label and stays aligned to the content margin. - renderer.drawText(UI_10_FONT_ID, contentX + 20, displayY, ">> Sync Progress", !isSelected); - } else { - const int tocIndex = tocIndexFromItemIndex(itemIndex); - auto item = epub->getTocItem(tocIndex); + auto item = epub->getTocItem(itemIndex); - // Indent per TOC level while keeping content within the gutter-safe region. - const int indentSize = contentX + 20 + (item.level - 1) * 15; - const std::string chapterName = - renderer.truncatedText(UI_10_FONT_ID, item.title.c_str(), contentWidth - 40 - indentSize); + // Indent per TOC level while keeping content within the gutter-safe region. + const int indentSize = contentX + 20 + (item.level - 1) * 15; + const std::string chapterName = + renderer.truncatedText(UI_10_FONT_ID, item.title.c_str(), contentWidth - 40 - indentSize); - renderer.drawText(UI_10_FONT_ID, indentSize, displayY, chapterName.c_str(), !isSelected); - } + renderer.drawText(UI_10_FONT_ID, indentSize, displayY, chapterName.c_str(), !isSelected); } const auto labels = mappedInput.mapLabels("« Back", "Select", "Up", "Down"); diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.h b/src/activities/reader/EpubReaderChapterSelectionActivity.h index 255f0cea..c43469d0 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.h +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.h @@ -26,22 +26,12 @@ class EpubReaderChapterSelectionActivity final : public ActivityWithSubactivity // This adapts automatically when switching between portrait and landscape. int getPageItems() const; - // Total items including sync options (top and bottom) + // Total TOC items count int getTotalItems() const; - // Check if sync option is available (credentials configured) - bool hasSyncOption() const; - - // Check if given item index is a sync option (first or last) - bool isSyncItem(int index) const; - - // Convert item index to TOC index (accounting for top sync option offset) - int tocIndexFromItemIndex(int itemIndex) const; - static void taskTrampoline(void* param); [[noreturn]] void displayTaskLoop(); void renderScreen(); - void launchSyncActivity(); public: explicit EpubReaderChapterSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 96307230..732cff5e 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/src/activities/reader/EpubReaderMenuActivity.cpp @@ -2,6 +2,7 @@ #include +#include "MappedInputManager.h" #include "components/UITheme.h" #include "fontIds.h" diff --git a/src/activities/reader/EpubReaderMenuActivity.h b/src/activities/reader/EpubReaderMenuActivity.h index b91d1a28..c24a610e 100644 --- a/src/activities/reader/EpubReaderMenuActivity.h +++ b/src/activities/reader/EpubReaderMenuActivity.h @@ -9,12 +9,11 @@ #include #include "../ActivityWithSubactivity.h" -#include "MappedInputManager.h" class EpubReaderMenuActivity final : public ActivityWithSubactivity { public: // Menu actions available from the reader menu. - enum class MenuAction { SELECT_CHAPTER, GO_TO_PERCENT, ROTATE_SCREEN, GO_HOME, DELETE_CACHE }; + enum class MenuAction { SELECT_CHAPTER, GO_TO_PERCENT, ROTATE_SCREEN, GO_HOME, SYNC, DELETE_CACHE }; explicit EpubReaderMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::string& title, const int currentPage, const int totalPages, const int bookProgressPercent, @@ -40,11 +39,10 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { }; // Fixed menu layout (order matters for up/down navigation). - const std::vector menuItems = {{MenuAction::SELECT_CHAPTER, "Go to Chapter"}, - {MenuAction::ROTATE_SCREEN, "Reading Orientation"}, - {MenuAction::GO_TO_PERCENT, "Go to %"}, - {MenuAction::GO_HOME, "Go Home"}, - {MenuAction::DELETE_CACHE, "Delete Book Cache"}}; + const std::vector menuItems = { + {MenuAction::SELECT_CHAPTER, "Go to Chapter"}, {MenuAction::ROTATE_SCREEN, "Reading Orientation"}, + {MenuAction::GO_TO_PERCENT, "Go to %"}, {MenuAction::GO_HOME, "Go Home"}, + {MenuAction::SYNC, "Sync Progress"}, {MenuAction::DELETE_CACHE, "Delete Book Cache"}}; int selectedIndex = 0; bool updateRequired = false;