Skip to content

Commit 3094249

Browse files
dcog989dcog989
committed
review feedback
1 parent 31fd6fd commit 3094249

File tree

3 files changed

+71
-51
lines changed

3 files changed

+71
-51
lines changed

‎Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs‎

Lines changed: 64 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class Main : ISettingProvider, IPlugin, IAsyncReloadable, IPluginI18n, IC
2626
private readonly CancellationTokenSource _cancellationTokenSource = new();
2727
private PeriodicTimer? _firefoxBookmarkTimer;
2828
private static readonly TimeSpan FirefoxPollingInterval = TimeSpan.FromHours(3);
29+
private readonly SemaphoreSlim _reloadGate = new(1, 1);
2930

3031
public void Init(PluginInitContext context)
3132
{
@@ -106,15 +107,23 @@ public List<Result> Query(Query query)
106107

107108
public async Task ReloadDataAsync()
108109
{
109-
var (bookmarks, discoveredFiles) = await _bookmarkLoader.LoadBookmarksAsync(_cancellationTokenSource.Token);
110+
await _reloadGate.WaitAsync(_cancellationTokenSource.Token);
111+
try
112+
{
113+
var (bookmarks, discoveredFiles) = await _bookmarkLoader.LoadBookmarksAsync(_cancellationTokenSource.Token);
110114

111-
// Atomically swap the list. This prevents the Query method from seeing a partially loaded list.
112-
Volatile.Write(ref _bookmarks, bookmarks);
115+
// Atomically swap the list. This prevents the Query method from seeing a partially loaded list.
116+
Volatile.Write(ref _bookmarks, bookmarks);
113117

114-
_bookmarkWatcher.UpdateWatchers(discoveredFiles);
118+
_bookmarkWatcher.UpdateWatchers(discoveredFiles);
115119

116-
// Fire and forget favicon processing to not block the UI
117-
_ = _faviconService.ProcessBookmarkFavicons(_bookmarks, _cancellationTokenSource.Token);
120+
// Fire and forget favicon processing to not block the UI
121+
_ = _faviconService.ProcessBookmarkFavicons(_bookmarks, _cancellationTokenSource.Token);
122+
}
123+
finally
124+
{
125+
_reloadGate.Release();
126+
}
118127
}
119128

120129
private void OnSettingsPropertyChanged(object? sender, PropertyChangedEventArgs e)
@@ -157,60 +166,69 @@ private void StartFirefoxBookmarkTimer()
157166

158167
private async Task ReloadFirefoxBookmarksAsync()
159168
{
160-
Context.API.LogInfo(nameof(Main), "Starting periodic reload of Firefox bookmarks.");
161-
162-
var firefoxLoaders = _bookmarkLoader.GetFirefoxBookmarkLoaders().ToList();
163-
if (!firefoxLoaders.Any())
169+
// Share the same gate to avoid conflicting with full reloads
170+
await _reloadGate.WaitAsync(_cancellationTokenSource.Token);
171+
try
164172
{
165-
Context.API.LogInfo(nameof(Main), "No Firefox bookmark loaders enabled, skipping reload.");
166-
return;
167-
}
173+
Context.API.LogInfo(nameof(Main), "Starting periodic reload of Firefox bookmarks.");
168174

169-
var tasks = firefoxLoaders.Select(async loader =>
170-
{
171-
var loadedBookmarks = new List<Bookmark>();
172-
try
173-
{
174-
await foreach (var bookmark in loader.GetBookmarksAsync(_cancellationTokenSource.Token))
175-
{
176-
loadedBookmarks.Add(bookmark);
177-
}
178-
return (Loader: loader, Bookmarks: loadedBookmarks, Success: true);
179-
}
180-
catch (OperationCanceledException)
175+
var firefoxLoaders = _bookmarkLoader.GetFirefoxBookmarkLoaders().ToList();
176+
if (!firefoxLoaders.Any())
181177
{
182-
return (Loader: loader, Bookmarks: new List<Bookmark>(), Success: false);
178+
Context.API.LogInfo(nameof(Main), "No Firefox bookmark loaders enabled, skipping reload.");
179+
return;
183180
}
184-
catch (Exception e)
181+
182+
var tasks = firefoxLoaders.Select(async loader =>
185183
{
186-
Context.API.LogException(nameof(Main), $"Failed to load bookmarks from {loader.Name}.", e);
187-
return (Loader: loader, Bookmarks: new List<Bookmark>(), Success: false);
188-
}
189-
});
184+
var loadedBookmarks = new List<Bookmark>();
185+
try
186+
{
187+
await foreach (var bookmark in loader.GetBookmarksAsync(_cancellationTokenSource.Token))
188+
{
189+
loadedBookmarks.Add(bookmark);
190+
}
191+
return (Loader: loader, Bookmarks: loadedBookmarks, Success: true);
192+
}
193+
catch (OperationCanceledException)
194+
{
195+
return (Loader: loader, Bookmarks: new List<Bookmark>(), Success: false);
196+
}
197+
catch (Exception e)
198+
{
199+
Context.API.LogException(nameof(Main), $"Failed to load bookmarks from {loader.Name}.", e);
200+
return (Loader: loader, Bookmarks: new List<Bookmark>(), Success: false);
201+
}
202+
});
190203

191-
var results = await Task.WhenAll(tasks);
192-
var successfulResults = results.Where(r => r.Success).ToList();
204+
var results = await Task.WhenAll(tasks);
205+
var successfulResults = results.Where(r => r.Success).ToList();
193206

194-
if (!successfulResults.Any())
195-
{
196-
Context.API.LogInfo(nameof(Main), "No Firefox bookmarks successfully reloaded.");
197-
return;
198-
}
207+
if (!successfulResults.Any())
208+
{
209+
Context.API.LogInfo(nameof(Main), "No Firefox bookmarks successfully reloaded.");
210+
return;
211+
}
199212

200-
var newFirefoxBookmarks = successfulResults.SelectMany(r => r.Bookmarks).ToList();
201-
var successfulLoaderNames = successfulResults.Select(r => r.Loader.Name).ToHashSet();
213+
var newFirefoxBookmarks = successfulResults.SelectMany(r => r.Bookmarks).ToList();
214+
var successfulLoaderNames = successfulResults.Select(r => r.Loader.Name).ToHashSet();
202215

203-
var currentBookmarks = Volatile.Read(ref _bookmarks);
216+
var currentBookmarks = Volatile.Read(ref _bookmarks);
204217

205-
var otherBookmarks = currentBookmarks.Where(b => !successfulLoaderNames.Any(name => b.Source.StartsWith(name, StringComparison.OrdinalIgnoreCase)));
218+
var otherBookmarks = currentBookmarks.Where(b => !successfulLoaderNames.Any(name => b.Source.StartsWith(name, StringComparison.OrdinalIgnoreCase)));
206219

207-
var newBookmarkList = otherBookmarks.Concat(newFirefoxBookmarks).Distinct().ToList();
220+
var newBookmarkList = otherBookmarks.Concat(newFirefoxBookmarks).Distinct().ToList();
208221

209-
Volatile.Write(ref _bookmarks, newBookmarkList);
222+
Volatile.Write(ref _bookmarks, newBookmarkList);
210223

211-
Context.API.LogInfo(nameof(Main), $"Periodic reload complete. Loaded {newFirefoxBookmarks.Count} Firefox bookmarks from {successfulLoaderNames.Count} sources.");
224+
Context.API.LogInfo(nameof(Main), $"Periodic reload complete. Loaded {newFirefoxBookmarks.Count} Firefox bookmarks from {successfulLoaderNames.Count} sources.");
212225

213-
_ = _faviconService.ProcessBookmarkFavicons(newFirefoxBookmarks, _cancellationTokenSource.Token);
226+
_ = _faviconService.ProcessBookmarkFavicons(newFirefoxBookmarks, _cancellationTokenSource.Token);
227+
}
228+
finally
229+
{
230+
_reloadGate.Release();
231+
}
214232
}
215233

216234
public Control CreateSettingPanel()

‎Plugins/Flow.Launcher.Plugin.BrowserBookmark/Services/FaviconService.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
namespace Flow.Launcher.Plugin.BrowserBookmark.Services;
1616

17+
public readonly record struct FaviconCandidate(string Url, int Score);
18+
1719
public partial class FaviconService : IDisposable
1820
{
1921
private readonly PluginInitContext _context;
@@ -28,8 +30,6 @@ public partial class FaviconService : IDisposable
2830
private ConcurrentDictionary<string, DateTime> _failedFetches = new(StringComparer.OrdinalIgnoreCase);
2931
private readonly string _failsFilePath;
3032
private readonly SemaphoreSlim _fileLock = new(1, 1);
31-
32-
public record struct FaviconCandidate(string Url, int Score);
3333
private record struct FetchResult(byte[]? PngData, int Size);
3434
private static readonly TimeSpan FailedFaviconCooldown = TimeSpan.FromHours(24);
3535

@@ -241,14 +241,14 @@ private static string GetCachePath(string url, string cacheDir)
241241
private static FetchResult SelectBestFavicon(FetchResult icoResult, FetchResult htmlResult)
242242
{
243243
var htmlValid = htmlResult.PngData != null;
244-
var icoValid = icoResult.PngData != null;
244+
var icoValid = icoResult.PngData != null;
245245

246246
if (htmlValid && htmlResult.Size >= ImageConverter.TargetIconSize) return htmlResult;
247-
if (icoValid && icoResult.Size >= ImageConverter.TargetIconSize) return icoResult;
247+
if (icoValid && icoResult.Size >= ImageConverter.TargetIconSize) return icoResult;
248248

249249
if (htmlValid && icoValid) return htmlResult.Size >= icoResult.Size ? htmlResult : icoResult;
250250
if (htmlValid) return htmlResult;
251-
if (icoValid) return icoResult;
251+
if (icoValid) return icoResult;
252252
return default;
253253
}
254254

‎Plugins/Flow.Launcher.Plugin.BrowserBookmark/Services/FirefoxBookmarkLoader.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ public async IAsyncEnumerable<Bookmark> GetBookmarksAsync([EnumeratorCancellatio
6666
if (File.Exists(shmPath))
6767
File.Copy(shmPath, tempDbPath + "-shm", true);
6868

69+
// Clear any partially-read results before fallback to avoid duplicates
70+
bookmarks.Clear();
6971
await ReadBookmarksFromDb(tempDbPath, bookmarks, cancellationToken);
7072
}
7173
catch (Exception copyEx)

0 commit comments

Comments
 (0)