Feature: Add disk based thumbnail caching system #17991
Conversation
…new row for Clear Cache button
There was a problem hiding this comment.
Changes are looking good as far as I can tell as of now. This code will still remain for the time being but we should use the built-in windows API to get thumbnail cache in the future. (Looks like IThumbnailProvider in the new Storage Abstraction API WindowsStorableHelpers.GetThumbnail already calls, nice)
Signed-off-by: Yair <39923744+yaira2@users.noreply.github.com>
ea17047 to
e0fb9ae
Compare
1962f3a to
14e6e26
Compare
| var fileSizeOnDisk = Win32Helper.GetFileSizeOnDisk(file); | ||
| totalSizeOnDisk += fileSizeOnDisk ?? 0; | ||
| } | ||
|
|
||
| return totalSizeOnDisk; | ||
| } | ||
| catch | ||
| { | ||
| return 0; | ||
| } | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| return await thumbnailService.GetThumbnailAsync( | ||
| path, | ||
| (int)size, | ||
| isFolder, | ||
| iconOptions); |
There was a problem hiding this comment.
Bug: FileThumbnailHelper.GetIconAsync lacks a CancellationToken parameter, preventing cancellation of long-running thumbnail generation tasks and potentially causing resource leaks when navigating away from folders.
Severity: MEDIUM
Suggested Fix
Propagate a CancellationToken through the call chain. Modify FileThumbnailHelper.GetIconAsync to accept a CancellationToken and pass it to IThumbnailService.GetThumbnailAsync. Ensure the underlying implementation in ShellApiThumbnailGenerator.GenerateAsync also respects the token to enable cancellation of the task.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/Files.App/Utils/Storage/Helpers/FileThumbnailHelper.cs#L22-L26
Potential issue: The `FileThumbnailHelper.GetIconAsync` method does not accept a
`CancellationToken`. Consequently, when called from `ShellViewModel.LoadThumbnailAsync`,
the cancellation token available in the view model cannot be propagated down to the
thumbnail generation service. The cancellation check
`cancellationToken.ThrowIfCancellationRequested()` occurs only *after* the `await` on
`GetIconAsync` completes. This means that if a user navigates away, the underlying Win32
Shell API calls will continue to run to completion, consuming CPU and STA thread
resources unnecessarily, which can lead to performance degradation.
Did we get this right? 👍 / 👎 to inform future reviews.
Resolved / Related Issues
Related to #4180
Addresses performance issues caused by repeated Shell API thumbnail generation by introducing a disk based LRU cache and a centralized ThumbnailService.
Steps used to test these changes