Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f34709f3 authored by Aurélien Pomini's avatar Aurélien Pomini
Browse files

Add a recycle check in WallpaperLocalColorExtractor

Alternative to ag/24403729.

First, this CL makes ImageWallpaper and WallpaperLocalColorExtractor
share the same lock.

Sharing the same lock simplifies the concurrency logic. Since both
ImageWallpaper and WallpaperLocalColorExtractor use the same
single-threaded @LongExecutor for their long operations, their
operations are already atomic but this is rather implicit. We make this
explicit by sharing the same lock.

Second, we add a recycle() check before calling createScaledBitmap in
WallpaperLocalColorExtractor, to avoid a concurrency bug where a new
image is loaded by ImageWallpaper before the color extraction of the
previous image could take place.

By sharing the lock, ImageWallpaper does (recycle(), mBitmap=newBitmap,
startColorExtraction(mBitmap)) atomically with regard to
WallpaperLocalColorExtractor. So when trying to extract colors, we
are guaranteed that either the bitmap is the most recent one, either it
is recycled.

Bug: 292492981
Test: atest ImageWallpaperTest
Test: atest WallpaperLocalColorExtractorTest
Change-Id: Ib5bccaaa7ebb4371feccfc7f5c8407410b8ba59b
parent 53a70ac4
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -133,6 +133,7 @@ public class ImageWallpaper extends WallpaperService {
            setShowForAllUsers(true);
            mWallpaperLocalColorExtractor = new WallpaperLocalColorExtractor(
                    mLongExecutor,
                    mLock,
                    new WallpaperLocalColorExtractor.WallpaperLocalColorExtractorCallback() {
                        @Override
                        public void onColorsProcessed(List<RectF> regions,
+10 −1
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ public class WallpaperLocalColorExtractor {
    private int mBitmapWidth = -1;
    private int mBitmapHeight = -1;

    private final Object mLock = new Object();
    private final Object mLock;

    private final List<RectF> mPendingRegions = new ArrayList<>();
    private final Set<RectF> mProcessedRegions = new ArraySet<>();
@@ -102,12 +102,15 @@ public class WallpaperLocalColorExtractor {
    /**
     * Creates a new color extractor.
     * @param longExecutor the executor on which the color extraction will be performed
     * @param lock the lock object to use for computing colors or processing the bitmap
     * @param wallpaperLocalColorExtractorCallback an interface to handle the callbacks from
     *                                        the color extractor.
     */
    public WallpaperLocalColorExtractor(@LongRunning Executor longExecutor,
            Object lock,
            WallpaperLocalColorExtractorCallback wallpaperLocalColorExtractorCallback) {
        mLongExecutor = longExecutor;
        mLock = lock;
        mWallpaperLocalColorExtractorCallback = wallpaperLocalColorExtractorCallback;
    }

@@ -149,6 +152,12 @@ public class WallpaperLocalColorExtractor {

    private void onBitmapChangedSynchronized(@NonNull Bitmap bitmap) {
        synchronized (mLock) {
            if (bitmap.isRecycled()) {
                // ImageWallpaper loaded a new bitmap before the extraction of the previous one
                // In that case, ImageWallpaper also scheduled the extraction of the next bitmap
                Log.i(TAG, "Wallpaper has changed; deferring color extraction");
                return;
            }
            if (bitmap.getWidth() <= 0 || bitmap.getHeight() <= 0) {
                Log.e(TAG, "Attempt to extract colors from an invalid bitmap");
                return;
+1 −0
Original line number Diff line number Diff line
@@ -109,6 +109,7 @@ public class WallpaperLocalColorExtractorTest extends SysuiTestCase {

        WallpaperLocalColorExtractor colorExtractor = new WallpaperLocalColorExtractor(
                mBackgroundExecutor,
                new Object(),
                new WallpaperLocalColorExtractor.WallpaperLocalColorExtractorCallback() {
                    @Override
                    public void onColorsProcessed(List<RectF> regions,