Fix download manager potentially not handling cancel requests p… (#7351)

Fix download manager potentially not handling cancel requests properly

Co-authored-by: Dean Herbert <pe@ppy.sh>

osu.Game.Tests/Online/TestSceneBeatmapManager.cs

@@ -0,0 +1,51 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Testing;
+using osu.Game.Beatmaps;
+using osu.Game.Overlays.Notifications;
+using osu.Game.Tests.Visual;
+
+namespace osu.Game.Tests.Online
+{
+    [HeadlessTest]
+    public class TestSceneBeatmapManager : OsuTestScene
+    {
+        private BeatmapManager beatmaps;
+        private ProgressNotification recentNotification;
+
+        private static readonly BeatmapSetInfo test_model = new BeatmapSetInfo { OnlineBeatmapSetID = 1 };
+
+        [BackgroundDependencyLoader]
+        private void load(BeatmapManager beatmaps)
+        {
+            this.beatmaps = beatmaps;
+
+            beatmaps.PostNotification = n => recentNotification = n as ProgressNotification;
+        }
+
+        [Test]
+        public void TestCancelDownloadFromRequest()
+        {
+            AddStep("download beatmap", () => beatmaps.Download(test_model));
+
+            AddStep("cancel download from request", () => beatmaps.GetExistingDownload(test_model).Cancel());
+
+            AddUntilStep("is removed from download list", () => beatmaps.GetExistingDownload(test_model) == null);
+            AddAssert("is notification cancelled", () => recentNotification.State == ProgressNotificationState.Cancelled);
+        }
+
+        [Test]
+        public void TestCancelDownloadFromNotification()
+        {
+            AddStep("download beatmap", () => beatmaps.Download(test_model));
+
+            AddStep("cancel download from notification", () => recentNotification.Close());
+
+            AddUntilStep("is removed from download list", () => beatmaps.GetExistingDownload(test_model) == null);
+            AddAssert("is notification cancelled", () => recentNotification.State == ProgressNotificationState.Cancelled);
+        }
+    }
+}

osu.Game/Database/DownloadableArchiveModelManager.cs

@@ -92,8 +92,6 @@ namespace osu.Game.Database
             notification.CancelRequested += () =>
             {
                 request.Cancel();
-                currentDownloads.Remove(request);
-                notification.State = ProgressNotificationState.Cancelled;
                 return true;
             };
 
@@ -109,11 +107,11 @@ namespace osu.Game.Database
             {
                 DownloadFailed?.Invoke(request);
 
-                if (error is OperationCanceledException) return;
-
-                notification.State = ProgressNotificationState.Cancelled;
-                Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!");
                 currentDownloads.Remove(request);
+                notification.State = ProgressNotificationState.Cancelled;
+
+                if (!(error is OperationCanceledException))
+                    Logger.Error(error, $"{HumanisedModelName.Titleize()} download failed!");
             }
         }