6 of 6 standards met
Motivation This PR aims to mitigate crash caused by thrown due to closable image reference being already closed when getting the image info. The crash is reported as #2826. On my end with version 3.6.0 I get following stack trace To mitigate the crash I moved reading image info before creating drawable from it and to be also surrounded by try-catch. Test Plan Unfortunately I can't provide a test plan since the exception is thrown most likely due to some race condition.
Repository: facebook/fresco. Description: An Android library for managing images and the memory they use. Stars: 17163, Forks: 3754. Primary language: Java. Languages: Java (49.8%), Kotlin (44.3%), C++ (4.1%), C (1.2%), Makefile (0.2%). License: MIT. Homepage: https://frescolib.org/ Latest release: v3.6.0 (1y ago). Open PRs: 14, open issues: 247. Last activity: 2d ago. Community health: 87%. Top contributors: oprisnik, defHLT, massimocarli, tyronen, kirwan, steelrooter, Andy-Wu25, foghina, lukkm, plamenko and others.
Last 12 weeks · 80 commits
Fixes #2849 Thanks for submitting a PR! Please read these instructions carefully: [x ] Explain the motivation for making this change. [ x] Provide a test plan demonstrating that the code is solid. [ x] Match the code formatting of the rest of the codebase. [ x] Target the branch Motivation (required) The current implementation of getTargetRenderTimeMs in DropFramesFrameScheduler.kt contains a logic error. In the summation loop, the code incorrectly uses the parameter frameNumber to fetch frame duration instead of the loop iterator i. The Problem: If we are calculating the target time for frame 10, the code currently adds the duration of frame 10 ten times. The Fix: The code should instead sum the durations of frames 0 through 9. This bug causes incorrect timing for animations with variable frame rates (non-uniform durations), leading to glitches when skipping or jumping to specific frames. Test Plan (required) Manual Code Review: Verified the loop variable change from to in Confirmed the method now correctly sums frame durations from index 0 to frameNumber-1 Build Verification: Build completed successfully with no compilation errors. Logic Verification Example: For a GIF with variable frame durations: Before fix (INCORRECT): returns ❌ Repeatedly adds duration of frame 4 instead of summing frames 0-3 After fix (CORRECT): returns ✅ Correctly sums durations of frames 0, 1, 2, and 3 Impact: This bug caused to calculate incorrect animation times, leading to: Wrong loop count calculations Premature animation termination Especially problematic for GIFs with non-uniform frame durations Existing Tests: The module contains existing unit tests in . All existing tests continue to pass with this fix. If you have added code that should be tested, add tests. Next Steps Sign the [CLA][2], if you haven't already. Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it. Make sure all tests pass** on [Circle CI][4]. PRs that break tests are unlikely to be merged. For more info, see the [Contributing guide][4]. [1]: https://medium.com/@martinkonicek/what-is-a-test-plan-8bfc840ec171#.y9lcuqqi9 [2]: https://code.facebook.com/cla [3]: http://circleci.com/gh/facebook/fresco [4]: https://github.com/facebook/fresco/blob/main/CONTRIBUTING.md
Description There is a critical bug in DropFramesFrameScheduler.getTargetRenderTimeMs() method where the loop uses the wrong variable (frameNumber instead of i), causing incorrect animation timing calculations when using jumpToFrame(). Affected Version Fresco version: all versions Module: animated-drawable File: com/facebook/fresco/animation/frame/DropFramesFrameScheduler.java Problem Code Location File: DropFramesFrameScheduler.java Current buggy code (line ~75-80): public long getTargetRenderTimeMs(int frameNumber) { long targetRenderTimeMs = 0; for (int i = 0; i < frameNumber; i++) { targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(frameNumber); // ❌ BUG: Should be 'i', not 'frameNumber' } return targetRenderTimeMs; } Root Cause The method incorrectly uses frameNumber (the parameter) instead of the loop variable i when calling getFrameDurationMs(). This causes it to repeatedly add the duration of the target frame instead of summing up durations of all frames leading up to it. Impact This bug severely affects GIF animations with non-uniform frame durations, especially when using AnimatedDrawable2.jumpToFrame(): 1. Incorrect time calculation: For a GIF with frames of different durations, jumping to a specific frame calculates wrong animation time 2. Animation stops prematurely: The wrong timing causes loopCount to be calculated incorrectly, leading to early animation termination 3. Loop count failure: Animations may stop even when configured for multiple or infinite loops Steps to Reproduce 1. Load a GIF with non-uniform frame durations (e.g., 18 frames at 30ms, 2 frames at 270ms) 2. Call animatedDrawable2.jumpToFrame(19) to jump to the last frame 3. Expected behavior: Animation should continue looping correctly 4. Actual behavior: Animation stops after 4-5 loops instead of continuing Example Calculation Error GIF Structure: Frames 0-17: 30ms each (total: 540ms) Frame 18: 270ms Frame 19: 270ms Total loop duration: 1080ms When calling jumpToFrame(19): ❌ Current buggy calculation: targetRenderTimeMs = 0; for (int i = 0; i < 19; i++) { targetRenderTimeMs += getFrameDurationMs(19); // Always adds 270ms } Result: 270ms × 19 = 5130ms // WRONG! ✅ Correct calculation: targetRenderTimeMs = 0; for (int i = 0; i < 19; i++) { targetRenderTimeMs += getFrameDurationMs(i); // Adds each frame's actual duration } Result: (30ms × 18) + 270ms = 810ms // CORRECT! The wrong timing (5130ms) causes the animation system to believe it's already in loop 4 (5130 / 1080 ≈ 4), leading to premature animation termination. Proposed Fix Change line ~78 from: targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(frameNumber); To: targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(i); Fixed Code public long getTargetRenderTimeMs(int frameNumber) { long targetRenderTimeMs = 0; for (int i = 0; i < frameNumber; i++) { targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(i); // ✅ FIXED: Use 'i' instead of 'frameNumber' } return targetRenderTimeMs; }
Summary Upgrade GitHub Actions to their latest versions to ensure compatibility with Node 24, as Node 20 will reach end-of-life in April 2026. Changes Context Per GitHub's announcement, Node 20 is being deprecated and runners will begin using Node 24 by default starting March 4th, 2026. Why this matters Node 20 EOL: April 2026 Node 24 default: March 4th, 2026 Action**: Update to latest action versions that support Node 24 Security Note Actions that were previously pinned to commit SHAs remain pinned to SHAs (updated to the latest release SHA) to maintain the security benefits of immutable references. Testing These changes only affect CI/CD workflow configurations and should not impact application functionality. The workflows should be tested by running them on a branch before merging.