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

Commit cd7c00c3 authored by Andrey Epin's avatar Andrey Epin
Browse files

Load icons and labels only once 

The current implementation triggers icon loading each time a view gets
bound and, as binding happens multiple times, icon and label loadings
are unnecessarily duplicated. In addition, as a loading task never gets
canceled, there’re cases (like in the associated ticket) when a view
gets rapidly bound to multiple positions and all the icon loadings
eventually catches up updating it, ending up in icons rapidly changing
on the screen for the view.
This change addresses this by simplifying the icon loading/binding
logic: a view is always bound to the current data and the task is only
updates the data with the missing piece of info once (and applied for
both icons and label loading tasks).
There is one extra change in ResolverActivity due to LoadIconTask
interface change and cancellation logic for the tasks.

Fix: 245934835
Test: Manual tests on a list which does not fit into the screen.
Test: Manual tests with arficial delays added into the tasks.
Test: atest FrameworksCoreTests:ResolverActivityTest
Test: atest FrameworksCoreTests:ChooserActivityTest
Change-Id: I76fe5663523232a74d4f505c535cd6dbcf7f8759
parent f12d6c28

core/java/com/android/internal/app/ChooserListAdapter.java

+13 −25
Original line number Diff line number Diff line
@@ -86,7 +86,6 @@ public class ChooserListAdapter extends ResolverListAdapter { 
    private final ChooserActivityLogger mChooserActivityLogger;



    private int mNumShortcutResults = 0;

    private Map<DisplayResolveInfo, LoadIconTask> mIconLoaders = new HashMap<>();

    private boolean mApplySharingAppLimits;



    // Reserve spots for incoming direct share targets by adding placeholders
@@ -265,10 +264,8 @@ public class ChooserListAdapter extends ResolverListAdapter { 
            return;

        }



        if (!(info instanceof DisplayResolveInfo)) {

        holder.bindLabel(info.getDisplayLabel(), info.getExtendedInfo(), alwaysShowSubLabel());

        holder.bindIcon(info);



        if (info instanceof SelectableTargetInfo) {

            // direct share targets should append the application name for a better readout

            DisplayResolveInfo rInfo = ((SelectableTargetInfo) info).getDisplayResolveInfo();
@@ -277,19 +274,10 @@ public class ChooserListAdapter extends ResolverListAdapter { 
            String contentDescription = String.join(" ", info.getDisplayLabel(),

                    extendedInfo != null ? extendedInfo : "", appName);

            holder.updateContentDescription(contentDescription);

            }

        } else {

        } else if (info instanceof DisplayResolveInfo) {

            DisplayResolveInfo dri = (DisplayResolveInfo) info;

            holder.bindLabel(dri.getDisplayLabel(), dri.getExtendedInfo(), alwaysShowSubLabel());

            LoadIconTask task = mIconLoaders.get(dri);

            if (task == null) {

                task = new LoadIconTask(dri, holder);

                mIconLoaders.put(dri, task);

                task.execute();

            } else {

                // The holder was potentially changed as the underlying items were

                // reshuffled, so reset the target holder

                task.setViewHolder(holder);

            if (!dri.hasDisplayIcon()) {

                loadIcon(dri);

            }

        }

core/java/com/android/internal/app/ResolverActivity.java

+13 −5
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ import android.content.pm.UserInfo; 
import android.content.res.Configuration;

import android.content.res.TypedArray;

import android.graphics.Insets;

import android.graphics.drawable.Drawable;

import android.net.Uri;

import android.os.Build;

import android.os.Bundle;
@@ -1475,14 +1476,21 @@ public class ResolverActivity extends Activity implements 
                mMultiProfilePagerAdapter.getActiveListAdapter().mDisplayList.get(0);

        boolean inWorkProfile = getCurrentProfile() == PROFILE_WORK;



        ResolverListAdapter inactiveAdapter = mMultiProfilePagerAdapter.getInactiveListAdapter();

        DisplayResolveInfo otherProfileResolveInfo = inactiveAdapter.mDisplayList.get(0);

        final ResolverListAdapter inactiveAdapter =

                mMultiProfilePagerAdapter.getInactiveListAdapter();

        final DisplayResolveInfo otherProfileResolveInfo = inactiveAdapter.mDisplayList.get(0);



        // Load the icon asynchronously

        ImageView icon = findViewById(R.id.icon);

        ResolverListAdapter.LoadIconTask iconTask = inactiveAdapter.new LoadIconTask(

                        otherProfileResolveInfo, new ResolverListAdapter.ViewHolder(icon));

        iconTask.execute();

        inactiveAdapter.new LoadIconTask(otherProfileResolveInfo) {

            @Override

            protected void onPostExecute(Drawable drawable) {

                if (!isDestroyed()) {

                    otherProfileResolveInfo.setDisplayIcon(drawable);

                    new ResolverListAdapter.ViewHolder(icon).bindIcon(otherProfileResolveInfo);

                }

            }

        }.execute();



        ((TextView) findViewById(R.id.open_cross_profile)).setText(

                getResources().getString(

core/java/com/android/internal/app/ResolverListAdapter.java

+54 −27
Original line number Diff line number Diff line
@@ -58,7 +58,10 @@ import com.android.internal.app.chooser.DisplayResolveInfo; 
import com.android.internal.app.chooser.TargetInfo;



import java.util.ArrayList;

import java.util.Collection;

import java.util.HashMap;

import java.util.List;

import java.util.Map;



public class ResolverListAdapter extends BaseAdapter {

    private static final String TAG = "ResolverListAdapter";
@@ -87,6 +90,8 @@ public class ResolverListAdapter extends BaseAdapter { 
    private Runnable mPostListReadyRunnable;

    private final boolean mIsAudioCaptureDevice;

    private boolean mIsTabLoaded;

    private final Map<DisplayResolveInfo, LoadIconTask> mIconLoaders = new HashMap<>();

    private final Map<DisplayResolveInfo, LoadLabelTask> mLabelLoaders = new HashMap<>();



    public ResolverListAdapter(Context context, List<Intent> payloadIntents,

            Intent[] initialIntents, List<ResolveInfo> rList,
@@ -636,26 +641,47 @@ public class ResolverListAdapter extends BaseAdapter { 
        if (info == null) {

            holder.icon.setImageDrawable(

                    mContext.getDrawable(R.drawable.resolver_icon_placeholder));

            holder.bindLabel("", "", false);

            return;

        }



        if (info instanceof DisplayResolveInfo

                && !((DisplayResolveInfo) info).hasDisplayLabel()) {

            getLoadLabelTask((DisplayResolveInfo) info, holder).execute();

        } else {

            holder.bindLabel(info.getDisplayLabel(), info.getExtendedInfo(), alwaysShowSubLabel());

        if (info instanceof DisplayResolveInfo) {

            DisplayResolveInfo dri = (DisplayResolveInfo) info;

            boolean hasLabel = dri.hasDisplayLabel();

            holder.bindLabel(

                    dri.getDisplayLabel(),

                    dri.getExtendedInfo(),

                    hasLabel && alwaysShowSubLabel());

            holder.bindIcon(info);

            if (!hasLabel) {

                loadLabel(dri);

            }

            if (!dri.hasDisplayIcon()) {

                loadIcon(dri);

            }

        }

    }



        if (info instanceof DisplayResolveInfo

                && !((DisplayResolveInfo) info).hasDisplayIcon()) {

            new LoadIconTask((DisplayResolveInfo) info, holder).execute();

        } else {

            holder.bindIcon(info);

    protected final void loadIcon(DisplayResolveInfo info) {

        LoadIconTask task = mIconLoaders.get(info);

        if (task == null) {

            task = new LoadIconTask((DisplayResolveInfo) info);

            mIconLoaders.put(info, task);

            task.execute();

        }

    }



    private void loadLabel(DisplayResolveInfo info) {

        LoadLabelTask task = mLabelLoaders.get(info);

        if (task == null) {

            task = createLoadLabelTask(info);

            mLabelLoaders.put(info, task);

            task.execute();

        }

    }



    protected LoadLabelTask getLoadLabelTask(DisplayResolveInfo info, ViewHolder holder) {

        return new LoadLabelTask(info, holder);

    protected LoadLabelTask createLoadLabelTask(DisplayResolveInfo info) {

        return new LoadLabelTask(info);

    }



    public void onDestroy() {
@@ -666,6 +692,16 @@ public class ResolverListAdapter extends BaseAdapter { 
        if (mResolverListController != null) {

            mResolverListController.destroy();

        }

        cancelTasks(mIconLoaders.values());

        cancelTasks(mLabelLoaders.values());

        mIconLoaders.clear();

        mLabelLoaders.clear();

    }



    private <T extends AsyncTask> void cancelTasks(Collection<T> tasks) {

        for (T task: tasks) {

            task.cancel(false);

        }

    }



    private static ColorMatrixColorFilter getSuspendedColorMatrix() {
@@ -883,11 +919,9 @@ public class ResolverListAdapter extends BaseAdapter { 


    protected class LoadLabelTask extends AsyncTask<Void, Void, CharSequence[]> {

        private final DisplayResolveInfo mDisplayResolveInfo;

        private final ViewHolder mHolder;



        protected LoadLabelTask(DisplayResolveInfo dri, ViewHolder holder) {

        protected LoadLabelTask(DisplayResolveInfo dri) {

            mDisplayResolveInfo = dri;

            mHolder = holder;

        }



        @Override
@@ -925,21 +959,22 @@ public class ResolverListAdapter extends BaseAdapter { 


        @Override

        protected void onPostExecute(CharSequence[] result) {

            if (mDisplayResolveInfo.hasDisplayLabel()) {

                return;

            }

            mDisplayResolveInfo.setDisplayLabel(result[0]);

            mDisplayResolveInfo.setExtendedInfo(result[1]);

            mHolder.bindLabel(result[0], result[1], alwaysShowSubLabel());

            notifyDataSetChanged();

        }

    }



    class LoadIconTask extends AsyncTask<Void, Void, Drawable> {

        protected final DisplayResolveInfo mDisplayResolveInfo;

        private final ResolveInfo mResolveInfo;

        private ViewHolder mHolder;



        LoadIconTask(DisplayResolveInfo dri, ViewHolder holder) {

        LoadIconTask(DisplayResolveInfo dri) {

            mDisplayResolveInfo = dri;

            mResolveInfo = dri.getResolveInfo();

            mHolder = holder;

        }



        @Override
@@ -953,17 +988,9 @@ public class ResolverListAdapter extends BaseAdapter { 
                mResolverListCommunicator.updateProfileViewButton();

            } else if (!mDisplayResolveInfo.hasDisplayIcon()) {

                mDisplayResolveInfo.setDisplayIcon(d);

                mHolder.bindIcon(mDisplayResolveInfo);

                // Notify in case view is already bound to resolve the race conditions on

                // low end devices

                notifyDataSetChanged();

            }

        }



        public void setViewHolder(ViewHolder holder) {

            mHolder = holder;

            mHolder.bindIcon(mDisplayResolveInfo);

        }

    }



    /**

core/tests/coretests/src/com/android/internal/app/ResolverWrapperAdapter.java

+4 −4
Original line number Diff line number Diff line
@@ -46,14 +46,14 @@ public class ResolverWrapperAdapter extends ResolverListAdapter { 
    }



    @Override

    protected LoadLabelTask getLoadLabelTask(DisplayResolveInfo info, ViewHolder holder) {

        return new LoadLabelWrapperTask(info, holder);

    protected LoadLabelTask createLoadLabelTask(DisplayResolveInfo info) {

        return new LoadLabelWrapperTask(info);

    }



    class LoadLabelWrapperTask extends LoadLabelTask {



        protected LoadLabelWrapperTask(DisplayResolveInfo dri, ViewHolder holder) {

            super(dri, holder);

        protected LoadLabelWrapperTask(DisplayResolveInfo dri) {

            super(dri);

        }



        @Override