fix: staled closure inside chrome_service

changelogs/fragments/8783.yml

@@ -0,0 +1,2 @@
+fix:
+- Staled closure inside chrome_service ([#8783](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8783))

src/core/public/chrome/chrome_service.test.ts

@@ -46,6 +46,7 @@ import { HeaderVariant } from './constants';
 
 class FakeApp implements App {
   public title: string;
+  public appRoute: string;
   public mount = () => () => {};
 
   constructor(
@@ -54,6 +55,7 @@ class FakeApp implements App {
     public headerVariant?: HeaderVariant
   ) {
     this.title = `${this.id} App`;
+    this.appRoute = this.id;
   }
 }
 const store = new Map();
@@ -78,12 +80,18 @@ function defaultStartDeps(availableApps?: App[]) {
     uiSettings: uiSettingsServiceMock.createStartContract(),
     overlays: overlayServiceMock.createStartContract(),
     workspaces: workspacesServiceMock.createStartContract(),
+    updateApplications: (() => {}) as (applications?: App[]) => void,
   };
 
   if (availableApps) {
-    deps.application.applications$ = new Rx.BehaviorSubject<Map<string, PublicAppInfo>>(
+    const applications$ = new Rx.BehaviorSubject<Map<string, PublicAppInfo>>(
       new Map(availableApps.map((app) => [app.id, getAppInfo(app) as PublicAppInfo]))
     );
+    deps.application.applications$ = applications$;
+    deps.updateApplications = (applications?: App[]) =>
+      applications$.next(
+        new Map(applications?.map((app) => [app.id, getAppInfo(app) as PublicAppInfo]))
+      );
   }
 
   return deps;
@@ -285,6 +293,30 @@ describe('start', () => {
                       ]
                   `);
     });
+
+    it('should use correct current app id to tell if hidden', async () => {
+      const apps = [new FakeApp('alpha', true), new FakeApp('beta', false)];
+      const startDeps = defaultStartDeps(apps);
+      const { navigateToApp } = startDeps.application;
+      const { chrome } = await start({ startDeps });
+      const visibleChangedArray: boolean[] = [];
+      const visible$ = chrome.getIsVisible$();
+      visible$.subscribe((visible) => visibleChangedArray.push(visible));
+
+      await navigateToApp('alpha');
+
+      await navigateToApp('beta');
+      startDeps.updateApplications(apps);
+
+      expect(visibleChangedArray).toMatchInlineSnapshot(`
+        Array [
+          false,
+          false,
+          true,
+          true,
+        ]
+      `);
+    });
   });
 
   describe('header variant', () => {

src/core/public/chrome/chrome_service.tsx

@@ -41,7 +41,7 @@ import {
   ReplaySubject,
   Subscription,
 } from 'rxjs';
-import { flatMap, map, takeUntil } from 'rxjs/operators';
+import { map, switchMap, takeUntil } from 'rxjs/operators';
 import { EuiLink } from '@elastic/eui';
 import { mountReactNode } from '../utils/mount';
 import { InternalApplicationStart } from '../application';
@@ -165,7 +165,13 @@ export class ChromeService {
       // in the sense that the chrome UI should not be displayed until a non-chromeless app is mounting or mounted
       of(true),
       application.currentAppId$.pipe(
-        flatMap((appId) =>
+        /**
+         * Using flatMap here will introduce staled closure issue.
+         * For example, when currentAppId$ is going through A -> B -> C and
+         * the application.applications$ just get changed in B, then it will always use B as the currentAppId
+         * even though the latest appId now is C.
+         */
+        switchMap((appId) =>
           application.applications$.pipe(
             map((applications) => {
               return !!appId && applications.has(appId) && !!applications.get(appId)!.chromeless;
@@ -184,7 +190,7 @@ export class ChromeService {
     this.headerVariantOverride$ = new BehaviorSubject<HeaderVariant | undefined>(undefined);
 
     const appHeaderVariant$ = application.currentAppId$.pipe(
-      flatMap((appId) =>
+      switchMap((appId) =>
         application.applications$.pipe(
           map(
             (applications) =>