From a0628cc521614835933db74ba5a2536a936629c5 Mon Sep 17 00:00:00 2001 From: Tony Wickham Date: Wed, 14 Oct 2015 15:23:04 -0700 Subject: [PATCH] Fix "The specified child already has a parent" IllegalStateException. The problem was due to a race condition between removing a prebound widget view from the drag layer and adding the same view to the workspace upon dropping it; if you let go of the widget immediately after picking it up, the latter happened before the former. Specifically, the flow was: long-click a widget --> drop --> remove the view from the drag layer if it's not null (it is, so nothing happens) --> the view is finally bound/inflated and added to the drag layer --> add the view to the workspace --> already has a parent. There are actually 2 problems here: one is that the bind/inflate is asynchronous, and can therefore happen after dropping the widget view being inflated, and the other is that the view is added to the workspace even though the transition has barely started (we usually ignore drops if the transition is less than half complete). It turns out that this second problem was also due to a race condition, this time between dropping a widget or app onto the workspace and calling LauncherStateTransitionAnimation.dispatchOnLauncherTransitionStart(). If the drop happened before the dispatch, as in the case of the crash, then the drop was accepted because the transition progress was still 1.0 from the previous transition. I fixed the first problem by removing the drag layer widget view in Launcher where it is potentially used instead of Workspace. And I fixed the second problem by setting mTransitionProgress to 0 in Workspace.onLauncherTransitionPrepare(). I also added some debugging logs. Bug: 23896857 Change-Id: I66944e6d3f23b70dea15f7fb01af0763a1bfcbda --- src/com/android/launcher3/CellLayout.java | 10 ++++--- src/com/android/launcher3/Launcher.java | 30 ++++++++++++------- src/com/android/launcher3/Workspace.java | 6 +--- .../widget/WidgetHostViewLoader.java | 27 +++++++++++++++-- .../widget/WidgetsContainerView.java | 15 ++++++---- 5 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/com/android/launcher3/CellLayout.java b/src/com/android/launcher3/CellLayout.java index 5539d9f8af..71a1df30fc 100644 --- a/src/com/android/launcher3/CellLayout.java +++ b/src/com/android/launcher3/CellLayout.java @@ -67,7 +67,8 @@ public class CellLayout extends ViewGroup implements BubbleTextShadowHandler { public static final int WORKSPACE_ACCESSIBILITY_DRAG = 2; public static final int FOLDER_ACCESSIBILITY_DRAG = 1; - static final String TAG = "CellLayout"; + private static final String TAG = "CellLayout"; + private static final boolean LOGD = false; private Launcher mLauncher; @Thunk int mCellWidth; @@ -242,9 +243,7 @@ public class CellLayout extends ViewGroup implements BubbleTextShadowHandler { // If an animation is started and then stopped very quickly, we can still // get spurious updates we've cleared the tag. Guard against this. if (outline == null) { - @SuppressWarnings("all") // suppress dead code warning - final boolean debug = false; - if (debug) { + if (LOGD) { Object val = animation.getAnimatedValue(); Log.d(TAG, "anim " + thisIndex + " update: " + val + ", isStopped " + anim.isStopped()); @@ -654,6 +653,9 @@ public class CellLayout extends ViewGroup implements BubbleTextShadowHandler { if (lp.cellVSpan < 0) lp.cellVSpan = mCountY; child.setId(childId); + if (LOGD) { + Log.d(TAG, "Adding view to ShortcutsAndWidgetsContainer: " + child); + } mShortcutsAndWidgets.addView(child, index, lp); if (markCells) markCellsAsOccupiedForView(child); diff --git a/src/com/android/launcher3/Launcher.java b/src/com/android/launcher3/Launcher.java index f90e09c39c..f5057c0b3d 100644 --- a/src/com/android/launcher3/Launcher.java +++ b/src/com/android/launcher3/Launcher.java @@ -46,7 +46,6 @@ import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; -import android.content.pm.ResolveInfo; import android.content.res.Configuration; import android.database.sqlite.SQLiteDatabase; import android.graphics.Bitmap; @@ -1728,14 +1727,14 @@ public class Launcher extends Activity mWorkspace.postDelayed(mBuildLayersRunnable, 500); final ViewTreeObserver.OnDrawListener listener = this; mWorkspace.post(new Runnable() { - public void run() { - if (mWorkspace != null && - mWorkspace.getViewTreeObserver() != null) { - mWorkspace.getViewTreeObserver(). - removeOnDrawListener(listener); - } + public void run() { + if (mWorkspace != null && + mWorkspace.getViewTreeObserver() != null) { + mWorkspace.getViewTreeObserver(). + removeOnDrawListener(listener); } - }); + } + }); return; } }); @@ -2226,8 +2225,11 @@ public class Launcher extends Activity mPendingAddInfo.dropPos = null; } - void addAppWidgetImpl(final int appWidgetId, final ItemInfo info, final + void addAppWidgetFromDropImpl(final int appWidgetId, final ItemInfo info, final AppWidgetHostView boundWidget, final LauncherAppWidgetProviderInfo appWidgetInfo) { + if (LOGD) { + Log.d(TAG, "Adding widget from drop"); + } addAppWidgetImpl(appWidgetId, info, boundWidget, appWidgetInfo, 0); } @@ -2335,8 +2337,14 @@ public class Launcher extends Activity AppWidgetHostView hostView = info.boundWidget; int appWidgetId; if (hostView != null) { + // In the case where we've prebound the widget, we remove it from the DragLayer + if (LOGD) { + Log.d(TAG, "Removing widget view from drag layer and setting boundWidget to null"); + } + getDragLayer().removeView(hostView); + appWidgetId = hostView.getAppWidgetId(); - addAppWidgetImpl(appWidgetId, info, hostView, info.info); + addAppWidgetFromDropImpl(appWidgetId, info, hostView, info.info); // Clear the boundWidget so that it doesn't get destroyed. info.boundWidget = null; @@ -2349,7 +2357,7 @@ public class Launcher extends Activity boolean success = mAppWidgetManager.bindAppWidgetIdIfAllowed( appWidgetId, info.info, options); if (success) { - addAppWidgetImpl(appWidgetId, info, null, info.info); + addAppWidgetFromDropImpl(appWidgetId, info, null, info.info); } else { mPendingAddWidgetInfo = info.info; Intent intent = new Intent(AppWidgetManager.ACTION_APPWIDGET_BIND); diff --git a/src/com/android/launcher3/Workspace.java b/src/com/android/launcher3/Workspace.java index 8d70c0b39a..23670c00c7 100644 --- a/src/com/android/launcher3/Workspace.java +++ b/src/com/android/launcher3/Workspace.java @@ -2060,6 +2060,7 @@ public class Workspace extends PagedView @Override public void onLauncherTransitionPrepare(Launcher l, boolean animated, boolean toWorkspace) { mIsSwitchingState = true; + mTransitionProgress = 0; // Invalidate here to ensure that the pages are rendered during the state change transition. invalidate(); @@ -3659,11 +3660,6 @@ public class Workspace extends PagedView Resources res = mLauncher.getResources(); final int duration = res.getInteger(R.integer.config_dropAnimMaxDuration) - 200; - // In the case where we've prebound the widget, we remove it from the DragLayer - if (finalView instanceof AppWidgetHostView && external) { - mLauncher.getDragLayer().removeView(finalView); - } - boolean isWidget = info.itemType == LauncherSettings.Favorites.ITEM_TYPE_APPWIDGET || info.itemType == LauncherSettings.Favorites.ITEM_TYPE_CUSTOM_APPWIDGET; if ((animationType == ANIMATE_INTO_POSITION_AND_RESIZE || external) && finalView != null) { diff --git a/src/com/android/launcher3/widget/WidgetHostViewLoader.java b/src/com/android/launcher3/widget/WidgetHostViewLoader.java index 461aebb6b8..5d3af52542 100644 --- a/src/com/android/launcher3/widget/WidgetHostViewLoader.java +++ b/src/com/android/launcher3/widget/WidgetHostViewLoader.java @@ -4,13 +4,13 @@ import android.appwidget.AppWidgetHostView; import android.appwidget.AppWidgetManager; import android.content.Context; import android.graphics.Rect; -import android.os.Build; import android.os.Bundle; import android.os.Handler; +import android.util.Log; import android.view.View; import com.android.launcher3.AppWidgetResizeFrame; -import com.android.launcher3.DragController.DragListener; +import com.android.launcher3.DragController; import com.android.launcher3.DragLayer; import com.android.launcher3.DragSource; import com.android.launcher3.Launcher; @@ -19,7 +19,9 @@ import com.android.launcher3.Utilities; import com.android.launcher3.compat.AppWidgetManagerCompat; import com.android.launcher3.util.Thunk; -public class WidgetHostViewLoader implements DragListener { +public class WidgetHostViewLoader implements DragController.DragListener { + private static final String TAG = "WidgetHostViewLoader"; + private static final boolean LOGD = false; /* Runnables to handle inflation and binding. */ @Thunk Runnable mInflateWidgetRunnable = null; @@ -48,6 +50,10 @@ public class WidgetHostViewLoader implements DragListener { @Override public void onDragEnd() { + if (LOGD) { + Log.d(TAG, "Cleaning up in onDragEnd()..."); + } + // Cleanup up preloading state. mLauncher.getDragController().removeDragListener(this); @@ -62,6 +68,9 @@ public class WidgetHostViewLoader implements DragListener { // The widget was inflated and added to the DragLayer -- remove it. if (mInfo.boundWidget != null) { + if (LOGD) { + Log.d(TAG, "...removing widget from drag layer"); + } mLauncher.getDragLayer().removeView(mInfo.boundWidget); mLauncher.getAppWidgetHost().deleteAppWidgetId(mInfo.boundWidget.getAppWidgetId()); mInfo.boundWidget = null; @@ -89,6 +98,9 @@ public class WidgetHostViewLoader implements DragListener { @Override public void run() { mWidgetLoadingId = mLauncher.getAppWidgetHost().allocateAppWidgetId(); + if (LOGD) { + Log.d(TAG, "Binding widget, id: " + mWidgetLoadingId); + } if(AppWidgetManagerCompat.getInstance(mLauncher).bindAppWidgetIdIfAllowed( mWidgetLoadingId, pInfo, options)) { @@ -101,6 +113,9 @@ public class WidgetHostViewLoader implements DragListener { mInflateWidgetRunnable = new Runnable() { @Override public void run() { + if (LOGD) { + Log.d(TAG, "Inflating widget, id: " + mWidgetLoadingId); + } if (mWidgetLoadingId == -1) { return; } @@ -120,11 +135,17 @@ public class WidgetHostViewLoader implements DragListener { lp.x = lp.y = 0; lp.customPosition = true; hostView.setLayoutParams(lp); + if (LOGD) { + Log.d(TAG, "Adding host view to drag layer"); + } mLauncher.getDragLayer().addView(hostView); mView.setTag(mInfo); } }; + if (LOGD) { + Log.d(TAG, "About to bind/inflate widget"); + } mHandler.post(mBindWidgetRunnable); return true; } diff --git a/src/com/android/launcher3/widget/WidgetsContainerView.java b/src/com/android/launcher3/widget/WidgetsContainerView.java index 64d33aa09a..b780f590b3 100644 --- a/src/com/android/launcher3/widget/WidgetsContainerView.java +++ b/src/com/android/launcher3/widget/WidgetsContainerView.java @@ -52,10 +52,9 @@ import com.android.launcher3.util.Thunk; * The widgets list view container. */ public class WidgetsContainerView extends BaseContainerView - implements View.OnLongClickListener, View.OnClickListener, DragSource{ - + implements View.OnLongClickListener, View.OnClickListener, DragSource { private static final String TAG = "WidgetsContainerView"; - private static final boolean DEBUG = false; + private static final boolean LOGD = false; /* Coefficient multiplied to the screen height for preloading widgets. */ private static final int PRELOAD_SCREEN_HEIGHT_MULTIPLE = 1; @@ -92,13 +91,14 @@ public class WidgetsContainerView extends BaseContainerView mDragController = mLauncher.getDragController(); mAdapter = new WidgetsListAdapter(context, this, this, mLauncher); mIconCache = (LauncherAppState.getInstance()).getIconCache(); - if (DEBUG) { + if (LOGD) { Log.d(TAG, "WidgetsContainerView constructor"); } } @Override protected void onFinishInflate() { + super.onFinishInflate(); mContent = findViewById(R.id.content); mView = (WidgetsRecyclerView) findViewById(R.id.widgets_list_view); mView.setAdapter(mAdapter); @@ -158,7 +158,7 @@ public class WidgetsContainerView extends BaseContainerView @Override public boolean onLongClick(View v) { - if (DEBUG) { + if (LOGD) { Log.d(TAG, String.format("onLonglick [v=%s]", v)); } // Return early if this is not initiated from a touch @@ -173,7 +173,7 @@ public class WidgetsContainerView extends BaseContainerView if (status && v.getTag() instanceof PendingAddWidgetInfo) { WidgetHostViewLoader hostLoader = new WidgetHostViewLoader(mLauncher, v); boolean preloadStatus = hostLoader.preloadWidget(); - if (DEBUG) { + if (LOGD) { Log.d(TAG, String.format("preloading widget [status=%s]", preloadStatus)); } mLauncher.getDragController().addDragListener(hostLoader); @@ -302,6 +302,9 @@ public class WidgetsContainerView extends BaseContainerView @Override public void onDropCompleted(View target, DragObject d, boolean isFlingToDelete, boolean success) { + if (LOGD) { + Log.d(TAG, "onDropCompleted"); + } if (isFlingToDelete || !success || (target != mLauncher.getWorkspace() && !(target instanceof DeleteDropTarget) && !(target instanceof Folder))) { // Exit spring loaded mode if we have not successfully dropped or have not handled the