Fix: Reset is_concealed_ in ThreadedImageDecoderProxy::Resume()#9218
Fix: Reset is_concealed_ in ThreadedImageDecoderProxy::Resume()#9218msolianko wants to merge 1 commit intoyoutube:25.lts.1+from
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug where image loaders would fail after a conceal/resume cycle by resetting the is_concealed_ flag. The fix is logical and well-implemented. I have provided one suggestion to further improve the code by using std::atomic<bool> for the flag, which would simplify the thread synchronization logic and be slightly more performant.
| { | ||
| base::AutoLock auto_lock(conceal_lock_); | ||
| is_concealed_ = false; | ||
| } |
There was a problem hiding this comment.
For a simple boolean flag like is_concealed_, using std::atomic<bool> is generally more efficient and can lead to simpler code than using a base::Lock. This avoids the overhead of lock acquisition and release.
If you change is_concealed_ to be a std::atomic<bool> and remove conceal_lock_, this entire block can be simplified to a single atomic store. This would also require updating other accesses to is_concealed_ (e.g., in Conceal() and Finish()) to use atomic operations instead of the lock.
is_concealed_ = false;2e4fd1b to
56af9e6
Compare
The ThreadedImageDecoderProxy::is_concealed_ flag was not reset in Resume() after being set to true by Conceal(). This prevented Finish() from posting the decode-complete task, causing image loaders to permanently fail to display images after a conceal/resume cycle. Reset the is_concealed_ flag in Resume() to restore normal decode completion and ensure images display correctly.
56af9e6 to
e338ca3
Compare
ThreadedImageDecoderProxy::Conceal() sets is_concealed_ = true, which causes Finish() to silently return without posting the decode-complete task to the load thread. This flag was never reset, so after any Blur→Conceal→Freeze→Unfreeze→Resume lifecycle cycle, all existing image loaders would permanently fail to complete — Loader::LoadComplete() would never be called and the image would never display.
The fix resets is_concealed_ to false in Resume() before re-initializing the underlying ImageDecoder, restoring normal Finish() behavior for loaders that survive a conceal/resume cycle.