diff --git a/Userland/Libraries/LibGUI/FileSystemModel.cpp b/Userland/Libraries/LibGUI/FileSystemModel.cpp index ca1027f81e..ee06238e94 100644 --- a/Userland/Libraries/LibGUI/FileSystemModel.cpp +++ b/Userland/Libraries/LibGUI/FileSystemModel.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -19,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -641,7 +643,16 @@ Icon FileSystemModel::icon_for(Node const& node) const return FileIconProvider::icon_for_path(node.full_path(), node.mode); } -static HashMap> s_thumbnail_cache; +using BitmapBackgroundAction = Threading::BackgroundAction>; + +// Mutex protected thumbnail cache data shared between threads. +struct ThumbnailCache { + // Null pointers indicate an image that couldn't be loaded due to errors. + HashMap> thumbnail_cache {}; + HashMap> loading_thumbnails {}; +}; + +static Threading::MutexProtected s_thumbnail_cache {}; static ErrorOr> render_thumbnail(StringView path) { @@ -658,55 +669,70 @@ static ErrorOr> render_thumbnail(StringView path) bool FileSystemModel::fetch_thumbnail_for(Node const& node) { - // See if we already have the thumbnail - // we're looking for in the cache. auto path = node.full_path(); - auto it = s_thumbnail_cache.find(path); - if (it != s_thumbnail_cache.end()) { - if (!(*it).value) - return false; - node.thumbnail = (*it).value; - return true; - } - // Otherwise, arrange to render the thumbnail - // in background and make it available later. + // See if we already have the thumbnail we're looking for in the cache. + auto was_in_cache = s_thumbnail_cache.with_locked([&](auto& cache) { + auto it = cache.thumbnail_cache.find(path); + if (it != cache.thumbnail_cache.end()) { + // Loading was unsuccessful. + if (!(*it).value) + return TriState::False; + // Loading was successful. + node.thumbnail = (*it).value; + return TriState::True; + } + // Loading is in progress. + if (cache.loading_thumbnails.contains(path)) + return TriState::False; + return TriState::Unknown; + }); + if (was_in_cache != TriState::Unknown) + return was_in_cache == TriState::True; + + // Otherwise, arrange to render the thumbnail in background and make it available later. - s_thumbnail_cache.set(path, nullptr); m_thumbnail_progress_total++; auto weak_this = make_weak_ptr(); - (void)Threading::BackgroundAction>>::construct( - [path](auto&) { - return render_thumbnail(path); - }, - - [this, path, weak_this](auto thumbnail_or_error) -> ErrorOr { - if (thumbnail_or_error.is_error()) { - s_thumbnail_cache.set(path, nullptr); - dbgln("Failed to load thumbnail for {}: {}", path, thumbnail_or_error.error()); - } else { - s_thumbnail_cache.set(path, thumbnail_or_error.release_value()); - } - - // The model was destroyed, no need to update - // progress or call any event handlers. - if (weak_this.is_null()) - return {}; - - m_thumbnail_progress++; - if (on_thumbnail_progress) - on_thumbnail_progress(m_thumbnail_progress, m_thumbnail_progress_total); - if (m_thumbnail_progress == m_thumbnail_progress_total) { - m_thumbnail_progress = 0; - m_thumbnail_progress_total = 0; - } - - did_update(UpdateFlag::DontInvalidateIndices); - return {}; + auto const action = [path](auto&) { + return render_thumbnail(path); + }; + auto const on_complete = [path, weak_this](auto thumbnail) -> ErrorOr { + s_thumbnail_cache.with_locked([path, thumbnail](auto& cache) { + cache.thumbnail_cache.set(path, thumbnail); + cache.loading_thumbnails.remove(path); }); + if (auto strong_this = weak_this.strong_ref(); !strong_this.is_null()) { + strong_this->m_thumbnail_progress++; + if (strong_this->on_thumbnail_progress) + strong_this->on_thumbnail_progress(strong_this->m_thumbnail_progress, strong_this->m_thumbnail_progress_total); + if (strong_this->m_thumbnail_progress == strong_this->m_thumbnail_progress_total) { + strong_this->m_thumbnail_progress = 0; + strong_this->m_thumbnail_progress_total = 0; + } + + strong_this->did_update(UpdateFlag::DontInvalidateIndices); + } + return {}; + }; + + auto const on_error = [path](Error error) -> void { + s_thumbnail_cache.with_locked([path, error = move(error)](auto& cache) { + if (error != Error::from_errno(ECANCELED)) { + cache.thumbnail_cache.set(path, nullptr); + dbgln("Failed to load thumbnail for {}: {}", path, error); + } + cache.loading_thumbnails.remove(path); + }); + }; + + s_thumbnail_cache.with_locked([path, action, on_complete, on_error](auto& cache) { + cache.loading_thumbnails.set(path, BitmapBackgroundAction::construct(move(action), move(on_complete), move(on_error))); + }); + return false; }