tweak thumbnails logic; don't delete thumbnails for now (fixes #4286)

src/ExternalViewers.cpp

@@ -259,7 +259,8 @@ static char* GetFoxitPathTemp() {
     if (path && file::Exists(path)) {
         return path;
     }
-    // Registry value for Foxit PDF Reader 12.1.3.15356 (The last version with Add Bookmark function without bugs in single-key accelerator)
+    // Registry value for Foxit PDF Reader 12.1.3.15356 (The last version with Add Bookmark function without bugs in
+    // single-key accelerator)
     keyName = R"(SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\FoxitPDFReader.exe)";
     path = ReadRegStrTemp(HKEY_LOCAL_MACHINE, keyName, "Path");
     if (path) {

src/FileHistory.cpp

@@ -2,13 +2,15 @@
 License: GPLv3 */
 
 #include "utils/BaseUtil.h"
+#include "utils/DirIter.h"
 #include "utils/FileUtil.h"
 #include "utils/ThreadUtil.h"
 #include "utils/UITask.h"
 #include "utils/WinUtil.h"
 
 #include "Settings.h"
 #include "GlobalPrefs.h"
+#include "FileThumbnails.h"
 #include "FileHistory.h"
 
 #include "utils/Log.h"
@@ -265,7 +267,57 @@ char* PopRecentlyClosedDocument() {
     return nullptr;
 }
 
-// ---- file existence check
+// --- thumbnail cache delete
+
+static bool shouldDeleteThumbnail = false;
+
+// TODO: https://github.com/sumatrapdfreader/sumatrapdf/issues/4286
+// Not sure why the behavior started changing after I re-wrote StrVec
+// is the issue that files are marked as isMissing in FileExistenceCheckerThread?
+// is it because we don't return enough itms if GetFrequencyOrder()? Is it a bug
+// in StrVec::Remove()?
+// either way, I just disabled deleting of stale thumbnail because it seems fishy
+// Should probably change the logic to: remove thumbnails for files marked as missing
+
+// removes thumbnails that don't belong to any frequently used item in file history
+void CleanUpThumbnailCache() {
+    const FileHistory& fileHistory = gFileHistory;
+    TempStr thumbsDir = GetThumbnailCacheDirTemp();
+    TempStr pattern = path::JoinTemp(thumbsDir, "*.png");
+
+    StrVec filePaths;
+    bool ok = CollectPathsFromDirectory(pattern, filePaths);
+    if (!ok || filePaths.IsEmpty()) {
+        return;
+    }
+
+    // remove files that should not be deleted
+    Vec<FileState*> list;
+    fileHistory.GetFrequencyOrder(list);
+    int n = 0;
+    for (auto& fs : list) {
+        if (n++ > kFileHistoryMaxFrequent * 2) {
+            break;
+        }
+        TempStr path = GetThumbnailPathTemp(fs->filePath);
+        if (!path) {
+            continue;
+        }
+        ok = filePaths.Remove(path);
+        if (!ok) {
+            logf("CleanUpThumbnailCache: failed to remove '%s'\n", path);
+        }
+    }
+
+    for (char* path : filePaths) {
+        logf("CleanUpThumbnailCache: deleting '%s'\n", path);
+        if (shouldDeleteThumbnail) {
+            file::Delete(path);
+        }
+    }
+}
+
+// --- file existence check
 
 struct FileExistenceData {
     FileExistenceData() = default;
@@ -324,12 +376,10 @@ static void FileExistenceCheckerThread(FileExistenceData* d) {
             continue;
         }
         d->missing.Append(path);
-        logf("FileExistenceChecker: missing '%s' at %d\n", path, i+1);
+        logf("FileExistenceChecker: missing '%s' at %d\n", path, i + 1);
     }
 
-    uitask::Post(TaskHideMissingFiles, [d] {
-        HideMissingFiles(d->missing);
-    });
+    uitask::Post(TaskHideMissingFiles, [d] { HideMissingFiles(d->missing); });
 }
 
 static void GetFilePathsToCheck(StrVec& toCheck) {
@@ -358,8 +408,6 @@ void RemoveNonExistentFilesAsync() {
         return;
     }
     logf("RemoveNonExistentFilesAsync: starting FileExistenceCheckerThread to check %d files\n", d->toCheck.Size());
-    auto fn = [d] {
-        FileExistenceCheckerThread(d); 
-    };
+    auto fn = [d] { FileExistenceCheckerThread(d); };
     RunAsync(fn, "FileExistenceThread");
 }

src/FileHistory.h

@@ -37,3 +37,4 @@ void RememberRecentlyClosedDocument(const char* path);
 char* PopRecentlyClosedDocument();
 void RemoveNonExistentFilesAsync();
 bool DocumentPathExists(const char* path);
+void CleanUpThumbnailCache();

src/FileThumbnails.cpp

@@ -18,10 +18,7 @@
 
 #include "utils/Log.h"
 
-constexpr const char* kThumbnailsDirName = "sumatrapdfcache";
-constexpr const char* kPngExt = "*.png";
-
-static char* GetThumbnailPathTemp(const char* filePath) {
+char* GetThumbnailPathTemp(const char* filePath) {
     // create a fingerprint of a (normalized) path for the file name
     // I'd have liked to also include the file's last modification time
     // in the fingerprint (much quicker than hashing the entire file's
@@ -39,7 +36,7 @@ static char* GetThumbnailPathTemp(const char* filePath) {
     CalcMD5Digest((u8*)path, str::Leni(path), digest);
     AutoFreeStr fingerPrint = str::MemToHex(digest, dimof(digest));
 
-    TempStr thumbsDir = AppGenDataFilenameTemp(kThumbnailsDirName);
+    TempStr thumbsDir = GetThumbnailCacheDirTemp();
     if (!thumbsDir) {
         return nullptr;
     }
@@ -48,44 +45,22 @@ static char* GetThumbnailPathTemp(const char* filePath) {
     return res;
 }
 
-void DeleteThumbnailCacheDirectory() {
+TempStr GetThumbnailCacheDirTemp() {
+    constexpr const char* kThumbnailsDirName = "sumatrapdfcache";
     TempStr thumbsDir = AppGenDataFilenameTemp(kThumbnailsDirName);
-    dir::RemoveAll(thumbsDir);
+    return thumbsDir;
 }
 
-// removes thumbnails that don't belong to any frequently used item in file history
-void CleanUpThumbnailCache(const FileHistory& fileHistory) {
-    TempStr thumbsDir = AppGenDataFilenameTemp(kThumbnailsDirName);
-    TempStr pattern = path::JoinTemp(thumbsDir, kPngExt);
-
-    StrVec filePaths;
-    bool ok = CollectPathsFromDirectory(pattern, filePaths);
-    if (!ok || filePaths.IsEmpty()) {
-        return;
-    }
-
-    // remove files that should not be deleted
-    Vec<FileState*> list;
-    fileHistory.GetFrequencyOrder(list);
-    int n = 0;
-    for (auto& fs : list) {
-        if (n++ > kFileHistoryMaxFrequent * 2) {
-            break;
-        }
-        TempStr path = GetThumbnailPathTemp(fs->filePath);
-        if (!path) {
-            continue;
-        }
-        ok = filePaths.Remove(path);
-        if (!ok) {
-            logf("CleanUpThumbnailCache: failed to remove '%s'\n", path);
-        }
-    }
+void DeleteThumbnailCacheDirectory() {
+    TempStr thumbsDir = GetThumbnailCacheDirTemp();
+    dir::RemoveAll(thumbsDir);
+}
 
-    for (char* path : filePaths) {
-        logf("CleanUpThumbnailCache: deleting '%s'\n", path);
-        file::Delete(path);
-    }
+void DeleteThumbnailForFile(const char* filePath) {
+    TempStr thumbPath = GetThumbnailPathTemp(filePath);
+    bool ok = file::Delete(thumbPath);
+    auto status = ok ? "ok" : "failed";
+    logf("DeleteThumbnailForFile: file::Remove('%s') %s\n", thumbPath, status);
 }
 
 bool LoadThumbnail(FileState* ds) {
@@ -144,19 +119,20 @@ void SaveThumbnail(FileState* ds) {
         return;
     }
 
-    char* path = ds->filePath;
-    char* bmpPath = GetThumbnailPathTemp(path);
-    if (!bmpPath) {
+    TempStr thumbnailPath = GetThumbnailPathTemp(ds->filePath);
+    if (!thumbnailPath) {
         return;
     }
-    char* thumbsPath = path::GetDirTemp(bmpPath);
-    if (dir::Create(thumbsPath)) {
-        ReportIf(!str::EndsWithI(bmpPath, ".png"));
-        Gdiplus::Bitmap bmp(ds->thumbnail->GetBitmap(), nullptr);
-        CLSID tmpClsid = GetEncoderClsid(L"image/png");
-        WCHAR* bmpPathW = ToWStrTemp(bmpPath);
-        bmp.Save(bmpPathW, &tmpClsid, nullptr);
+    if (!dir::CreateForFile(thumbnailPath)) {
+        logf("SaveThumbnail: dir::CreateForFile('%s') failed, file path: '%s'\n", thumbnailPath, ds->filePath);
+        ReportIfQuick(true);
     }
+    ReportIfQuick(!str::EndsWithI(thumbnailPath, ".png"));
+
+    Gdiplus::Bitmap bmp(ds->thumbnail->GetBitmap(), nullptr);
+    CLSID tmpClsid = GetEncoderClsid(L"image/png");
+    TempWStr pathW = ToWStrTemp(thumbnailPath);
+    bmp.Save(pathW, &tmpClsid, nullptr);
 }
 
 void RemoveThumbnail(FileState* ds) {

src/FileThumbnails.h

@@ -11,5 +11,7 @@ void SetThumbnail(FileState* ds, RenderedBitmap* bmp);
 void SaveThumbnail(FileState* ds);
 void RemoveThumbnail(FileState* ds);
 
+TempStr GetThumbnailCacheDirTemp();
+char* GetThumbnailPathTemp(const char* filePath);
+void DeleteThumbnailForFile(const char* path);
 void DeleteThumbnailCacheDirectory();
-void CleanUpThumbnailCache(const FileHistory& fileHistory);

src/Menu.cpp

@@ -1703,14 +1703,15 @@ void OnAboutContextMenu(MainWindow* win, int x, int y) {
     }
 
     if (CmdForgetSelectedDocument == cmd) {
-        if (fs->favorites->size() > 0) {
-            // just hide documents with favorites
+        if (!fs->favorites->IsEmpty()) {
+            // only hide documents with favorites
             gFileHistory.MarkFileInexistent(fs->filePath, true);
         } else {
             gFileHistory.Remove(fs);
             DeleteDisplayState(fs);
         }
-        CleanUpThumbnailCache(gFileHistory);
+        DeleteThumbnailForFile(fs->filePath);
+        SaveSettings();
         win->DeleteToolTip();
         win->RedrawAll(true);
         return;

src/SumatraPDF.cpp

@@ -3662,7 +3662,7 @@ static void ShowOptionsDialog(HWND hwnd) {
 
     if (!gGlobalPrefs->rememberOpenedFiles) {
         gFileHistory.Clear(true);
-        CleanUpThumbnailCache(gFileHistory);
+        DeleteThumbnailCacheDirectory();
     }
     UpdateDocumentColors();
 

src/SumatraStartup.cpp

@@ -1356,7 +1356,7 @@ int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE, LPSTR, int) {
 
     exitCode = RunMessageLoop();
     SafeCloseHandle(&hMutex);
-    CleanUpThumbnailCache(gFileHistory);
+    CleanUpThumbnailCache();
 
 Exit:
     logf("Exiting with exit code: %d\n", exitCode);