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

Commit 8786f3cf authored by Vadim Tryshev's avatar Vadim Tryshev
Browse files

Clarifying and stabilizing drag & drop 

The existing Shelf DND crashes if profile or app removal happens
while dragging from outside of shelf. This is because in this case
shelf temporary adds a null placeholder enter to the model, and
doesn’t check for nulls in user-removed and app-removed callbacks.

Adding checking for nulls.
In addition, since adding nulls muds the contract between the
shelf view and its model, I’m getting rid of adding null entries
to the model. Nulls are used on the view side, but never leak to
the model.

Because of that, the state of the view (which may have nulls) and the
mode may not match each other during the drag, I introduce storing
app data in views, as a tag. Null tag means a drag placeholder.

Now that the state of the model doesn’t always follow the state
of the shelf via add/remove operations, it makes sense to introduce
set-all/get-all calls in the model, instead of add/remove/get(i)
calls.

Again, all these changes, even though relatively massive, add clarity
into the view/model contract by eliminating strange states where the
model can have nulls in certain positions.

Also fixing a case when removing an app while dragging it happily
creates an icon.

Bug: 20024603
Change-Id: Ie4e617ccf9278708d64ee100265a4858d1227aed
parent 6c0773ec

packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarApps.java

+60 −47
Original line number Diff line number Diff line
@@ -46,6 +46,9 @@ import android.widget.Toast; 
import com.android.internal.content.PackageMonitor;

import com.android.systemui.R;



import java.util.ArrayList;

import java.util.List;



/**

 * Container for application icons that appear in the navigation bar. Their appearance is similar

 * to the launcher hotseat. Clicking an icon launches the associated activity. A long click will
@@ -73,12 +76,12 @@ class NavigationBarApps extends LinearLayout { 


    // This view has two roles:

    // 1) If the drag started outside the pinned apps list, it is a placeholder icon with a null

    // data model entry.

    // tag.

    // 2) If the drag started inside the pinned apps list, it is the icon for the app being dragged

    // with the associated data model entry.

    // with the associated AppInfo tag.

    // The icon is set invisible for the duration of the drag, creating a visual space for a drop.

    // When the user is not dragging this member is null.

    private View mDragView;

    private ImageView mDragView;



    private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {

        @Override
@@ -171,8 +174,10 @@ class NavigationBarApps extends LinearLayout { 


    private void removeIfUnlauncheable(String packageName, UserHandle user) {

        // Remove icons for all apps that match a package that perhaps became unlauncheable.

        for(int i = sAppsModel.getAppCount() - 1; i >= 0; --i) {

            AppInfo appInfo = sAppsModel.getApp(i);

        for(int i = getChildCount() - 1; i >= 0; --i) {

            View child = getChildAt(i);

            AppInfo appInfo = (AppInfo)child.getTag();

            if (appInfo == null) continue;  // Skip the drag placeholder.

            if (!appInfo.getUser().equals(user)) continue;



            ComponentName appComponentName = appInfo.getComponentName();
@@ -183,8 +188,9 @@ class NavigationBarApps extends LinearLayout { 
            }



            removeViewAt(i);

            sAppsModel.removeApp(i);

            sAppsModel.savePrefs();

        }

        if (getChildCount() != sAppsModel.getApps().size()) {

            saveApps();

        }

    }
@@ -229,12 +235,13 @@ class NavigationBarApps extends LinearLayout { 
        // Remove any existing icon buttons.

        removeAllViews();



        int appCount = sAppsModel.getAppCount();

        List<AppInfo> apps = sAppsModel.getApps();

        int appCount = apps.size();

        for (int i = 0; i < appCount; i++) {

            ImageView button = createAppButton();

            AppInfo app = apps.get(i);

            ImageView button = createAppButton(app);

            addView(button);



            AppInfo app = sAppsModel.getApp(i);

            CharSequence appLabel = getAppLabel(mPackageManager, app.getComponentName());

            button.setContentDescription(appLabel);
@@ -243,17 +250,33 @@ class NavigationBarApps extends LinearLayout { 
        }

    }



    /**

     * Saves apps stored in app icons into the data model.

     */

    private void saveApps() {

        List<AppInfo> apps = new ArrayList<AppInfo>();

        int childCount = getChildCount();

        for (int i = 0; i != childCount; ++i) {

            View child = getChildAt(i);

            AppInfo appInfo = (AppInfo)child.getTag();

            if (appInfo == null) continue;  // Skip the drag placeholder.

            apps.add(appInfo);

        }

        sAppsModel.setApps(apps);

    }



    /**

     * Creates a new ImageView for a launcher activity, inflated from

     * R.layout.navigation_bar_app_item.

     */

    private ImageView createAppButton() {

    private ImageView createAppButton(AppInfo appInfo) {

        ImageView button = (ImageView) mLayoutInflater.inflate(

                R.layout.navigation_bar_app_item, this, false /* attachToRoot */);

        button.setOnClickListener(new AppClickListener());

        // TODO: Ripple effect. Use either KeyButtonRipple or the default ripple background.

        button.setOnLongClickListener(new AppLongClickListener());

        button.setOnDragListener(new AppIconDragListener());

        button.setTag(appInfo);

        return button;

    }
@@ -261,10 +284,9 @@ class NavigationBarApps extends LinearLayout { 
    private class AppLongClickListener implements View.OnLongClickListener {

        @Override

        public boolean onLongClick(View v) {

            mDragView = v;

            ImageView icon = (ImageView) v;

            AppInfo app = sAppsModel.getApp(indexOfChild(v));

            startAppDrag(icon, app);

            mDragView = (ImageView) v;

            AppInfo app = (AppInfo) v.getTag();

            startAppDrag(mDragView, app);

            return true;

        }

    }
@@ -368,13 +390,11 @@ class NavigationBarApps extends LinearLayout { 
    }



    /**

     * Creates a blank icon-sized View to create an empty space during a drag. Also creates a data

     * model entry so the rest of the code can assume it is reordering existing entries.

     * Creates a blank icon-sized View to create an empty space during a drag.

     */

    private ImageView createPlaceholderDragView(int index) {

        ImageView button = createAppButton();

        ImageView button = createAppButton(null);

        addView(button, index);

        sAppsModel.addApp(index, null /* name */);

        return button;

    }
@@ -409,10 +429,6 @@ class NavigationBarApps extends LinearLayout { 
        //   the view at targetIndex will therefore place the app *after* the target.

        removeView(mDragView);

        addView(mDragView, targetIndex);



        // Update the data model.

        AppInfo app = sAppsModel.removeApp(dragViewIndex);

        sAppsModel.addApp(targetIndex, app);

    }



    private boolean onDrop(DragEvent event) {
@@ -423,23 +439,21 @@ class NavigationBarApps extends LinearLayout { 
            return true;

        }



        // If this was an existing app being dragged then end the drag.

        int dragViewIndex = indexOfChild(mDragView);

        if (sAppsModel.getApp(dragViewIndex) != null) {

            endDrag();

            return true;

        }



        // The drag view was a placeholder. Unpack the drop.

        AppInfo appInfo = getAppFromDragEvent(event);

        if (appInfo == null) {

            // This wasn't a valid drop. Clean up the placeholder and model.

            // This wasn't a valid drop. Clean up the placeholder.

            removePlaceholderDragViewIfNeeded();

            return false;

        }



        // The drop had valid data. Update the placeholder with a real activity and icon.

        updateAppAt(dragViewIndex, appInfo);

        // If this was an existing app being dragged then end the drag.

        if (mDragView.getTag() != null) {

            endDrag();

            return true;

        }



        // The drop had valid data. Convert the placeholder to a real icon.

        updateApp(mDragView, appInfo);

        endDrag();

        return true;

    }
@@ -448,7 +462,7 @@ class NavigationBarApps extends LinearLayout { 
    private void endDrag() {

        mDragView.setVisibility(View.VISIBLE);

        mDragView = null;

        sAppsModel.savePrefs();

        saveApps();

    }



    /** Returns an app info from a DragEvent, or null if the data wasn't valid. */
@@ -488,13 +502,12 @@ class NavigationBarApps extends LinearLayout { 
    }



    /** Updates the app at a given view index. */

    private void updateAppAt(int index, AppInfo appInfo) {

        sAppsModel.setApp(index, appInfo);

        ImageView button = (ImageView) getChildAt(index);

    private void updateApp(ImageView button, AppInfo appInfo) {

        button.setTag(appInfo);

        new GetActivityIconTask(mPackageManager, button).execute(appInfo);

    }



    /** Removes the empty placeholder view and cleans up the data model. */

    /** Removes the empty placeholder view. */

    private void removePlaceholderDragViewIfNeeded() {

        // If the drag has ended already there is nothing to do.

        if (mDragView == null) {
@@ -502,7 +515,6 @@ class NavigationBarApps extends LinearLayout { 
        }

        int index = indexOfChild(mDragView);

        removeViewAt(index);

        sAppsModel.removeApp(index);

        endDrag();

    }
@@ -548,7 +560,7 @@ class NavigationBarApps extends LinearLayout { 
    private class AppClickListener implements View.OnClickListener {

        @Override

        public void onClick(View v) {

            AppInfo appInfo = sAppsModel.getApp(indexOfChild(v));

            AppInfo appInfo = (AppInfo)v.getTag();

            Intent launchIntent = sAppsModel.buildAppLaunchIntent(appInfo);

            if (launchIntent == null) {

                Toast.makeText(
@@ -577,15 +589,16 @@ class NavigationBarApps extends LinearLayout { 
    }



    private void onManagedProfileRemoved(UserHandle removedProfile) {

        boolean removedApps = false;

        for(int i = sAppsModel.getAppCount() - 1; i >= 0; --i) {

            AppInfo appInfo = sAppsModel.getApp(i);

        for(int i = getChildCount() - 1; i >= 0; --i) {

            View view = getChildAt(i);

            AppInfo appInfo = (AppInfo)view.getTag();

            if (appInfo == null) return;  // Skip the drag placeholder.

            if (!appInfo.getUser().equals(removedProfile)) continue;



            removeViewAt(i);

            sAppsModel.removeApp(i);

            removedApps = true;

        }

        if (removedApps) sAppsModel.savePrefs();

        if (getChildCount() != sAppsModel.getApps().size()) {

            saveApps();

        }

    }

}

packages/SystemUI/src/com/android/systemui/statusbar/phone/NavigationBarAppsModel.java

+9 −23
Original line number Diff line number Diff line
@@ -78,7 +78,7 @@ class NavigationBarAppsModel { 
    private final SharedPreferences mPrefs;



    // Apps are represented as an ordered list of app infos.

    private final List<AppInfo> mApps = new ArrayList<AppInfo>();

    private List<AppInfo> mApps = new ArrayList<AppInfo>();



    // Id of the current user.

    private int mCurrentUserId = -1;
@@ -231,33 +231,19 @@ class NavigationBarAppsModel { 
        edit.apply();

    }



    /** Returns the number of apps. */

    public int getAppCount() {

        return mApps.size();

    /** Returns the list of apps. */

    public List<AppInfo> getApps() {

        return mApps;

    }



    /** Returns the app at the given index. */

    public AppInfo getApp(int index) {

        return mApps.get(index);

    }



    /** Adds the app before the given index. */

    public void addApp(int index, AppInfo appInfo) {

        mApps.add(index, appInfo);

    }



    /** Sets the app at the given index. */

    public void setApp(int index, AppInfo appInfo) {

        mApps.set(index, appInfo);

    }



    /** Remove the app at the given index. */

    public AppInfo removeApp(int index) {

        return mApps.remove(index);

    /** Sets the list of apps and saves it. */

    public void setApps(List<AppInfo> apps) {

        mApps = apps;

        savePrefs();

    }



    /** Saves the current model to disk. */

    public void savePrefs() {

    private void savePrefs() {

        SharedPreferences.Editor edit = mPrefs.edit();

        int appCount = mApps.size();

        edit.putInt(userPrefixed(APP_COUNT_PREF), appCount);

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NavigationBarAppsModelTest.java

+21 −17
Original line number Diff line number Diff line
@@ -218,11 +218,12 @@ public class NavigationBarAppsModelTest extends AndroidTestCase { 
    /** Tests initializing the model from SharedPreferences. */

    public void testInitializeFromPrefs() {

        initializeModelFromPrefs();

        assertEquals(2, mModel.getAppCount());

        assertEquals("package1/class1", mModel.getApp(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(4), mModel.getApp(0).getUser());

        assertEquals("package2/class2", mModel.getApp(1).getComponentName().flattenToString());

        assertEquals(new UserHandle(5), mModel.getApp(1).getUser());

        List<AppInfo> apps = mModel.getApps();

        assertEquals(2, apps.size());

        assertEquals("package1/class1", apps.get(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(4), apps.get(0).getUser());

        assertEquals("package2/class2", apps.get(1).getComponentName().flattenToString());

        assertEquals(new UserHandle(5), apps.get(1).getUser());

    }



    /** Tests initializing the model when the SharedPreferences aren't available. */
@@ -247,11 +248,12 @@ public class NavigationBarAppsModelTest extends AndroidTestCase { 


        // Setting the user should load the installed activities.

        mModel.setCurrentUser(2);

        assertEquals(2, mModel.getAppCount());

        assertEquals("package1/class1", mModel.getApp(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(2), mModel.getApp(0).getUser());

        assertEquals("package2/class2", mModel.getApp(1).getComponentName().flattenToString());

        assertEquals(new UserHandle(2), mModel.getApp(1).getUser());

        List<AppInfo> apps = mModel.getApps();

        assertEquals(2, apps.size());

        assertEquals("package1/class1", apps.get(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(2), apps.get(0).getUser());

        assertEquals("package2/class2", apps.get(1).getComponentName().flattenToString());

        assertEquals(new UserHandle(2), apps.get(1).getUser());

        InOrder order = inOrder(mMockEdit);

        order.verify(mMockEdit).apply();

        order.verify(mMockEdit).putInt("222|app_count", 2);
@@ -293,9 +295,10 @@ public class NavigationBarAppsModelTest extends AndroidTestCase { 


        // Initializing the model should load from prefs and skip the missing one.

        mModel.setCurrentUser(2);

        assertEquals(1, mModel.getAppCount());

        assertEquals("package0/class0", mModel.getApp(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(5), mModel.getApp(0).getUser());

        List<AppInfo> apps = mModel.getApps();

        assertEquals(1, apps.size());

        assertEquals("package0/class0", apps.get(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(5), apps.get(0).getUser());

        InOrder order = inOrder(mMockEdit);

        order.verify(mMockEdit).putInt("222|app_count", 1);

        order.verify(mMockEdit).putString("222|app_0", "package0/class0");
@@ -333,9 +336,10 @@ public class NavigationBarAppsModelTest extends AndroidTestCase { 


        // Initializing the model should load from prefs and skip the unlauncheable one.

        mModel.setCurrentUser(2);

        assertEquals(1, mModel.getAppCount());

        assertEquals("package0/class0", mModel.getApp(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(5), mModel.getApp(0).getUser());

        List<AppInfo> apps = mModel.getApps();

        assertEquals(1, apps.size());

        assertEquals("package0/class0", apps.get(0).getComponentName().flattenToString());

        assertEquals(new UserHandle(5), apps.get(0).getUser());



        // Once an unlauncheable app is detected, the model should save all apps excluding the

        // unlauncheable one.
@@ -350,7 +354,7 @@ public class NavigationBarAppsModelTest extends AndroidTestCase { 
    public void testSavePrefs() {

        initializeModelFromPrefs();



        mModel.savePrefs();

        mModel.setApps(mModel.getApps());

        verify(mMockEdit).putInt("222|app_count", 2);

        verify(mMockEdit).putString("222|app_0", "package1/class1");

        verify(mMockEdit).putLong("222|app_user_0", 444L);