in ChmThumbnailTask prevent callsing saveThumbnail more than once (for …

…#4519)
sumatrapdfreader · Sep 10, 2024 · a412fb0 · a412fb0
1 parent e6686bd
commit a412fb0
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 18 deletions.
diff --git a/src/ChmModel.cpp b/src/ChmModel.cpp
@@ -624,15 +624,16 @@ struct ChmThumbnailTask : HtmlWindowCallback {
     ChmFile* doc = nullptr;
     HWND hwnd = nullptr;
     HtmlWindow* hw = nullptr;
+    bool didSave = false;
     Size size;
-    const OnBitmapRendered* saveThumbnail;
+    const OnBitmapRendered* saveThumbnail = nullptr;
     AutoFreeStr homeUrl;
     Vec<ByteSlice> data;
     CRITICAL_SECTION docAccess;
 
     ChmThumbnailTask(ChmFile* doc, HWND hwnd, Size size, const OnBitmapRendered* saveThumbnail);
     ~ChmThumbnailTask() override;
-    void CreateThumbnail(HtmlWindow* hw);
+    void StartCreateThumbnail(HtmlWindow* hw);
     bool OnBeforeNavigate(const char*, bool newWindow) override;
     void OnDocumentComplete(const char* url) override;
     ByteSlice GetDataForUrl(const char* url) override;
@@ -650,6 +651,7 @@ ChmThumbnailTask::ChmThumbnailTask(ChmFile* doc, HWND hwnd, Size size, const OnB
     this->hwnd = hwnd;
     this->size = size;
     this->saveThumbnail = saveThumbnail;
+    this->didSave = false;
     InitializeCriticalSection(&docAccess);
 }
 
@@ -669,7 +671,7 @@ bool ChmThumbnailTask::OnBeforeNavigate(const char*, bool newWindow) {
     return !newWindow;
 }
 
-void ChmThumbnailTask::CreateThumbnail(HtmlWindow* hw) {
+void ChmThumbnailTask::StartCreateThumbnail(HtmlWindow* hw) {
     this->hw = hw;
     homeUrl.Set(strconv::AnsiToUtf8(doc->GetHomePath()));
     if (*homeUrl == '/') {
@@ -690,17 +692,27 @@ void ChmThumbnailTask::OnDocumentComplete(const char* url) {
     if (url && *url == '/') {
         url++;
     }
-    if (str::Eq(url, homeUrl)) {
-        Rect area(0, 0, size.dx * 2, size.dy * 2);
-        HBITMAP hbmp = hw->TakeScreenshot(area, size);
-        if (hbmp) {
-            RenderedBitmap* bmp = new RenderedBitmap(hbmp, size);
-            saveThumbnail->Call(bmp);
-        }
-        // TODO: why is destruction on the UI thread necessary?
-        auto fn = MkFunc0<ChmThumbnailTask>(SafeDeleteChmThumbnailTask, this);
-        uitask::Post(fn, "SafeDeleteChmThumbnailTask");
+    if (!str::Eq(url, homeUrl)) {
+        return;
+    }
+    logf("ChmThumbnailTask::OnDocumentComplete: '%s'\n", url);
+    if (didSave) {
+        // maybe prevent crash generating .chm thumbnails
+        // /~https://github.com/sumatrapdfreader/sumatrapdf/issues/4519
+        ReportIfQuick(didSave);
+        return;
+    }
+    Rect area(0, 0, size.dx * 2, size.dy * 2);
+    HBITMAP hbmp = hw->TakeScreenshot(area, size);
+    if (hbmp) {
+        RenderedBitmap* bmp = new RenderedBitmap(hbmp, size);
+        saveThumbnail->Call(bmp);
     }
+    // delay deleting because ~ChmThumbnailTask() deletes HtmlWindow
+    // and we're currently processing HtmlWindow messages
+    // TODO: it's possible we still have timing issue
+    auto fn = MkFunc0<ChmThumbnailTask>(SafeDeleteChmThumbnailTask, this);
+    uitask::Post(fn, "SafeDeleteChmThumbnailTask");
 }
 
 void ChmThumbnailTask::OnLButtonDown() {
@@ -738,7 +750,7 @@ static void CreateChmThumbnail(const char* path, const Size& size, const OnBitma
         return;
     }
     // is deleted in ChmThumbnailTask::OnDocumentComplete
-    thumbnailTask->CreateThumbnail(hw);
+    thumbnailTask->StartCreateThumbnail(hw);
 }
 
 // Create a thumbnail of chm document by loading it again and rendering

diff --git a/src/SumatraPDF.cpp b/src/SumatraPDF.cpp
@@ -738,7 +738,7 @@ static void CreateThumbnailFinish(CreateThumbnailData* d) {
 static void CreateThumbnailOnBitmapRendered(CreateThumbnailData* d, RenderedBitmap* bmp) {
     d->bmp = bmp;
     auto fn = MkFunc0<CreateThumbnailData>(CreateThumbnailFinish, d);
-    uitask::Post(fn, "TaskSetThumbnail");
+    uitask::PostOptimized(fn, "TaskSetThumbnail");
 }
 
 static void CreateThumbnailForFile(MainWindow* win, FileState* ds) {

diff --git a/src/utils/UITask.cpp b/src/utils/UITask.cpp
@@ -74,7 +74,6 @@ void Post(const Func0& f, Kind kind) {
     PostMessageW(gTaskDispatchHwnd, wmExecTask, (WPARAM)kind, (LPARAM)func);
 } // NOLINT
 
-#if 0
 void PostOptimized(const Func0& f, Kind kind) {
     if (IsGUIThread(FALSE)) {
         // if we're already on ui thread, execute immediately
@@ -84,6 +83,5 @@ void PostOptimized(const Func0& f, Kind kind) {
     }
     Post(f, kind);
 } // NOLINT
-#endif
 
 } // namespace uitask
diff --git a/src/utils/UITask.h b/src/utils/UITask.h
@@ -11,5 +11,6 @@ void Destroy();
 void DrainQueue();
 
 void Post(const Func0& fn, Kind kind = nullptr);
-// void PostOptimized(const Func0& fn, Kind kind = nullptr);
+void PostOptimized(const Func0& fn, Kind kind = nullptr);
+
 } // namespace uitask