Address comments

superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { act } from 'react';
 import * as reduxHooks from 'react-redux';
 import { Provider } from 'react-redux';
 import { createStore } from 'redux';
@@ -180,4 +181,78 @@ describe('OAuth2RedirectMessage Component', () => {
 
     expect(reRunQuery).not.toHaveBeenCalled();
   });
+
+  test('dispatches only once when both BroadcastChannel and storage signals arrive', async () => {
+    render(setup());
+
+    simulateBroadcastMessage({ tabId: 'tabId' });
+    simulateStorageMessage({ tabId: 'tabId' });
+
+    await waitFor(() => {
+      expect(reRunQuery).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  test('falls back to storage events when BroadcastChannel construction throws', async () => {
+    (global as any).BroadcastChannel = jest.fn().mockImplementation(() => {
+      throw new Error('blocked');
+    });
+
+    render(setup());
+
+    simulateStorageMessage({ tabId: 'tabId' });
+
+    await waitFor(() => {
+      expect(reRunQuery).toHaveBeenCalledWith({ sql: 'SELECT * FROM table' });
+    });
+  });
+
+  test('re-processes a later signal when the first arrived before state was ready', async () => {
+    const initialState = {
+      sqlLab: {
+        queries: {},
+        queryEditors: [{ id: 'editor-id' }],
+        tabHistory: ['editor-id'],
+      },
+      explore: { slice: { slice_id: 123 } },
+      charts: { '1': {}, '2': {} },
+      dashboardInfo: { id: 'dashboard-id' },
+    };
+    const dynamicStore = createStore(
+      (state: any = initialState, action: any) => {
+        if (action.type === 'SET_READY') {
+          return {
+            ...state,
+            sqlLab: {
+              ...state.sqlLab,
+              queries: { 'query-id': { sql: 'SELECT * FROM table' } },
+              queryEditors: [{ id: 'editor-id', latestQueryId: 'query-id' }],
+            },
+          };
+        }
+        return state;
+      },
+    );
+
+    render(
+      <Provider store={dynamicStore}>
+        <OAuth2RedirectMessage {...defaultProps} />
+      </Provider>,
+    );
+
+    // First signal arrives before the SQL Lab query state is populated;
+    // nothing dispatches and `handled` must NOT be flipped.
+    simulateBroadcastMessage({ tabId: 'tabId' });
+    expect(reRunQuery).not.toHaveBeenCalled();
+
+    // Query state becomes available, then the storage fallback signal fires.
+    act(() => {
+      dynamicStore.dispatch({ type: 'SET_READY' });
+    });
+    simulateStorageMessage({ tabId: 'tabId' });
+
+    await waitFor(() => {
+      expect(reRunQuery).toHaveBeenCalledWith({ sql: 'SELECT * FROM table' });
+    });
+  });
 });

superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useEffect } from 'react';
+import { useEffect, useRef } from 'react';
 
 import { useDispatch, useSelector } from 'react-redux';
 import { QueryEditor, SqlLabRootState } from 'src/SqlLab/types';
@@ -99,29 +99,66 @@ export function OAuth2RedirectMessage({
 
   const dispatch = useDispatch();
 
+  // `chartList` is rebuilt from `Object.keys` on every store update; keeping
+  // the listener state in a ref avoids tearing down/recreating the
+  // BroadcastChannel on each render and the race window that comes with it.
+  const latestStateRef = useRef({
+    source,
+    query,
+    chartId,
+    chartList,
+    dashboardId,
+  });
+  latestStateRef.current = {
+    source,
+    query,
+    chartId,
+    chartList,
+    dashboardId,
+  };
+
   useEffect(() => {
+    // Guard against duplicate dispatches if both the BroadcastChannel and the
+    // storage fallback ever deliver the same completion.
+    let handled = false;
     const handleOAuthComplete = (tabId?: string) => {
-      if (tabId !== extra.tab_id) {
+      if (tabId !== extra.tab_id || handled) {
         return;
       }
-      if (source === 'sqllab' && query) {
+      const {
-        dispatch(reRunQuery(query));
+        source: src,
-      } else if (source === 'explore' && chartId) {
+        query: q,
-        dispatch(triggerQuery(true, chartId));
+        chartId: cId,
-      } else if (source === 'dashboard') {
+        chartList: cList,
-        dispatch(onRefresh(chartList.map(Number), true, 0, dashboardId));
+        dashboardId: dId,
+      } = latestStateRef.current;
+      // Only flip `handled` once a dispatch actually runs so a later fallback
+      // signal can still succeed if the first arrived before state was ready
+      // (e.g. `query` still null in SQL Lab).
+      if (src === 'sqllab' && q) {
+        handled = true;
+        dispatch(reRunQuery(q));
+      } else if (src === 'explore' && cId) {
+        handled = true;
+        dispatch(triggerQuery(true, cId));
+      } else if (src === 'dashboard') {
+        handled = true;
+        dispatch(onRefresh(cList.map(Number), true, 0, dId));
       }
     };
 
-    const channel =
+    // `BroadcastChannel` may exist on `window` but throw at construction time
-      typeof BroadcastChannel !== 'undefined'
+    // in restricted contexts; fall back to the storage listener if so.
-        ? new BroadcastChannel(OAUTH_CHANNEL_NAME)
+    let channel: BroadcastChannel | null = null;
-        : null;
+    if (typeof BroadcastChannel !== 'undefined') {
-
+      try {
-    if (channel) {
+        channel = new BroadcastChannel(OAUTH_CHANNEL_NAME);
-      channel.onmessage = event => {
+        channel.onmessage = event => {
-        handleOAuthComplete(event.data?.tabId);
+          handleOAuthComplete(event.data?.tabId);
-      };
+        };
+      } catch {
+        channel = null;
+      }
     }
 
     const handleStorage = (event: StorageEvent) => {
@@ -143,7 +180,7 @@ export function OAuth2RedirectMessage({
       window.removeEventListener('storage', handleStorage);
       channel?.close();
     };
-  }, [source, extra.tab_id, dispatch, query, chartId, chartList, dashboardId]);
+  }, [extra.tab_id, dispatch]);
 
   const body = (
     <p>

superset/templates/superset/oauth2.html

@@ -25,13 +25,23 @@ under the License.
   <body>
     <script nonce="{{ macros.get_nonce() }}">
       const message = { tabId: '{{ tab_id }}' };
+      // Emit on both channels: postMessage success doesn't guarantee the original
+      // tab's listener was attached. The receiver dedupes via a `handled` flag.
       if (typeof BroadcastChannel !== 'undefined') {
-        const channel = new BroadcastChannel('oauth');
+        try {
-        channel.postMessage(message);
+          const channel = new BroadcastChannel('oauth');
-        channel.close();
+          channel.postMessage(message);
+          channel.close();
+        } catch (e) {
+          // ignore; storage path below still fires
+        }
+      }
+      try {
+        localStorage.setItem('oauth2_auth_complete', JSON.stringify(message));
+        localStorage.removeItem('oauth2_auth_complete');
+      } catch (e) {
+        // storage may be unavailable (e.g. blocked or restricted); ignore
       }
-      localStorage.setItem('oauth2_auth_complete', JSON.stringify(message));
-      localStorage.removeItem('oauth2_auth_complete');
       window.close();
     </script>
     <p>You can close this window and re-run the query.</p>