Merge pull request #37772 from bernhardoj/fix/37710-app-crash-when-ex…

…panding-video Fix crash when expanding video player (cherry picked from commit 4ae0b42)
Expensify · Mar 5, 2024 · 0302094 · 0302094
1 parent 9ede329
commit 0302094
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 13 deletions.
diff --git a/src/components/Attachments/AttachmentView/index.js b/src/components/Attachments/AttachmentView/index.js
@@ -101,7 +101,7 @@ function AttachmentView({
     isHovered,
     optionalVideoDuration,
 }) {
-    const {updateCurrentlyPlayingURL, currentVideoPlayerRef} = usePlaybackContext();
+    const {updateCurrentlyPlayingURL} = usePlaybackContext();
     const theme = useTheme();
     const styles = useThemeStyles();
     const StyleUtils = useStyleUtils();
@@ -115,17 +115,6 @@ function AttachmentView({
         updateCurrentlyPlayingURL(isVideo ? source : null);
     }, [isFocused, isVideo, source, updateCurrentlyPlayingURL, file, isUsedInAttachmentModal]);
 
-    // This should ensure we clean up any video references when closing the attachment modal as these only existed here in memory during attachment preview.
-    useEffect(
-        () => () => {
-            if (!isVideo) {
-                return;
-            }
-            currentVideoPlayerRef.current = null;
-        },
-        [isVideo, currentVideoPlayerRef],
-    );
-
     const [imageError, setImageError] = useState(false);
 
     useNetwork({onReconnect: () => setImageError(false)});

diff --git a/src/components/VideoPlayer/BaseVideoPlayer.js b/src/components/VideoPlayer/BaseVideoPlayer.js
@@ -60,6 +60,7 @@ function BaseVideoPlayer({
     const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
     const isCurrentlyURLSet = currentlyPlayingURL === url;
     const isUploading = _.some(CONST.ATTACHMENT_LOCAL_URL_PREFIX, (prefix) => url.startsWith(prefix));
+    const shouldUseSharedVideoElementRef = useRef(shouldUseSharedVideoElement);
 
     const togglePlayCurrentVideo = useCallback(() => {
         videoResumeTryNumber.current = 0;
@@ -145,8 +146,27 @@ function BaseVideoPlayer({
     }, [currentVideoPlayerRef, handleFullscreenUpdate, handlePlaybackStatusUpdate]);
 
     useEffect(() => {
+        if (!isUploading) {
+            return;
+        }
+        // If we are uploading a new video, we want to immediately set the video player ref.
         currentVideoPlayerRef.current = videoPlayerRef.current;
-    }, [url, currentVideoPlayerRef]);
+    }, [url, currentVideoPlayerRef, isUploading]);
+
+    useEffect(() => {
+        shouldUseSharedVideoElementRef.current = shouldUseSharedVideoElement;
+    }, [shouldUseSharedVideoElement]);
+
+    useEffect(
+        () => () => {
+            if (shouldUseSharedVideoElementRef.current) {
+                return;
+            }
+            // If it's not a shared video player, clear the video player ref.
+            currentVideoPlayerRef.current = null;
+        },
+        [currentVideoPlayerRef],
+    );
 
     // update shared video elements
     useEffect(() => {