fix: raise OnViewClosed when a view's life cycle does not end normally

  MVCManager.ShowAsync raised OnViewClosed after the layer branch returned, so the
  event was skipped whenever that branch threw. Cancellation is the common case --
  it is caught and swallowed a few lines below -- but every layer branch tears the
  view down in its own finally block, so the view is hidden either way. What
  subscribers were left with was an OnViewShowed that never resolved.

  UpscalingController is the visible casualty: it counts open UIs and lowers the
  render scale while any of them is up, so a single cancelled show leaves
  currentUIOpened above zero and the scale pinned at the UI-open value for the
  rest of the session. SidebarController and ChatMainSharedAreaController track
  state off the same pair of events.

  Moving the invocation into a finally block pairs every OnViewShowed with exactly
  one OnViewClosed. Nothing can now raise it twice: the call was moved rather than
  added, and the early return for a controller that is not hidden sits ahead of
  the try, where neither event fires.

  MVCManagerShould covers the four ways ShowAsync can end -- the life cycle
  completing, being cancelled, failing with anything else, and the early return --
  asserting the exact event sequence in each rather than a count, so a duplicate
  fails the same way a missing one does. Events are only exercised on the
  Fullscreen layer; the finally block is in ShowAsync, shared by all four.

Author: popuz

Explorer/Assets/DCL/Infrastructure/MVC/Manager/MVCManager.cs

@@ -128,14 +128,17 @@ public async UniTask ShowAsync<TView, TInputData>(ShowCommand<TView, TInputData>
                         await ShowOverlayAsync(command, controller, ct);
                         break;
                 }
-
-                OnViewClosed?.Invoke(controller);
             }
             catch (OperationCanceledException)
             {
                 // TODO (Vit) : handle revert of command. Proposal - extend WizardCommands interface with Revert method and call it in case of cancellation.
                 ReportHub.LogWarning(ReportCategory.MVC, $"ShowAsync was cancelled for {controller.GetType()}");
             }
+            finally
+            {
+                // Raised here so that every OnViewShowed is followed by exactly one OnViewClosed
+                OnViewClosed?.Invoke(controller);
+            }
         }
 
         private async UniTask ShowOverlayAsync<TView, TInputData>(ShowCommand<TView, TInputData> command, IController controller, CancellationToken ct)

Explorer/Assets/DCL/Infrastructure/MVC/Tests/MVCManagerShould.cs

@@ -14,6 +14,8 @@ public class MVCManagerShould
         private IWindowsStackManager windowsStackManager;
         private MVCManager mvcManager;
         private IPopupCloserView popupCloserView;
+        private List<IController> showed;
+        private List<IController> closed;
 
         [SetUp]
         public void Setup()
@@ -22,6 +24,12 @@ public void Setup()
             windowsStackManager.PushFullscreen(Arg.Any<IController>()).Returns(new FullscreenPushInfo(new List<(IController, int)>(), new CanvasOrdering(), new UniTaskCompletionSource()));
             popupCloserView = Substitute.For<IPopupCloserView>();
             mvcManager = new MVCManager(windowsStackManager, new CancellationTokenSource(), popupCloserView);
+
+            showed = new List<IController>();
+            closed = new List<IController>();
+
+            mvcManager.OnViewShowed += showed.Add;
+            mvcManager.OnViewClosed += closed.Add;
         }
 
         [Test]
@@ -84,6 +92,91 @@ public async Task Show(CanvasOrdering.SortingLayer layer)
                     break;
             }
         }
+
+        [Test]
+        public async Task RaiseViewClosedWhenTheViewLifeCycleIsCancelled()
+        {
+            IController<ITestView, TestInputData> controller = Substitute.For<IController<ITestView, TestInputData>>();
+            controller.Layer.Returns(CanvasOrdering.SortingLayer.Fullscreen);
+
+            var viewLifeCycle = new UniTaskCompletionSource();
+
+            controller.LaunchViewLifeCycleAsync(Arg.Any<CanvasOrdering>(), Arg.Any<TestInputData>(), Arg.Any<CancellationToken>())
+                      .Returns(viewLifeCycle.Task);
+
+            mvcManager.RegisterController(controller);
+
+            UniTask show = mvcManager.ShowAsync(new ShowCommand<ITestView, TestInputData>());
+
+            viewLifeCycle.TrySetCanceled();
+
+            await show;
+
+            // Guards the test itself: without this the assertion below would also hold for a run that
+            // never entered the show path at all.
+            Assert.That(showed, Is.EqualTo(new[] { controller }));
+
+            // ShowFullScreenAsync tears the view down in a finally block and pops it off the stack, so a
+            // cancelled life cycle still ends with the view hidden. Subscribers are told about every other
+            // way it can end, and they have no other signal to pair with OnViewShowed.
+            Assert.That(closed, Is.EqualTo(new[] { controller }));
+        }
+
+        [Test]
+        public async Task RaiseViewClosedOnceWhenTheViewLifeCycleCompletes()
+        {
+            IController<ITestView, TestInputData> controller = Substitute.For<IController<ITestView, TestInputData>>();
+            controller.Layer.Returns(CanvasOrdering.SortingLayer.Fullscreen);
+
+            mvcManager.RegisterController(controller);
+
+            await mvcManager.ShowAsync(new ShowCommand<ITestView, TestInputData>());
+
+            // Exact sequences rather than counts: the show path leaves the view hidden the ordinary way
+            // too, and the pairing is one to one, so a second OnViewClosed here would be a duplicate.
+            Assert.That(showed, Is.EqualTo(new[] { controller }));
+            Assert.That(closed, Is.EqualTo(new[] { controller }));
+        }
+
+        [Test]
+        public void RaiseViewClosedWhenTheViewLifeCycleFails()
+        {
+            IController<ITestView, TestInputData> controller = Substitute.For<IController<ITestView, TestInputData>>();
+            controller.Layer.Returns(CanvasOrdering.SortingLayer.Fullscreen);
+
+            var viewLifeCycle = new UniTaskCompletionSource();
+
+            controller.LaunchViewLifeCycleAsync(Arg.Any<CanvasOrdering>(), Arg.Any<TestInputData>(), Arg.Any<CancellationToken>())
+                      .Returns(viewLifeCycle.Task);
+
+            mvcManager.RegisterController(controller);
+
+            UniTask show = mvcManager.ShowAsync(new ShowCommand<ITestView, TestInputData>());
+
+            viewLifeCycle.TrySetException(new InvalidOperationException());
+
+            // Only cancellation is swallowed, so this one travels out of ShowAsync — and the view is torn
+            // down on the way, which is what subscribers are told about.
+            Assert.ThrowsAsync<InvalidOperationException>(async () => await show);
+            Assert.That(closed, Is.EqualTo(new[] { controller }));
+        }
+
+        [Test]
+        public async Task RaiseNoViewEventsForAControllerThatIsNotHidden()
+        {
+            IController<ITestView, TestInputData> controller = Substitute.For<IController<ITestView, TestInputData>>();
+            controller.Layer.Returns(CanvasOrdering.SortingLayer.Fullscreen);
+            controller.State.Returns(ControllerState.ViewFocused);
+
+            mvcManager.RegisterController(controller);
+
+            await mvcManager.ShowAsync(new ShowCommand<ITestView, TestInputData>());
+
+            // ShowAsync returns before showing anything in this case, so there is no session to report
+            // either end of.
+            Assert.That(showed, Is.Empty);
+            Assert.That(closed, Is.Empty);
+        }
     }
 
     public class TestInputData { }