Commit 52516a4c authored by tedchoc's avatar tedchoc Committed by Commit bot

Fix crash when showing multiple popups.

The problem is that calling hide/cancel on a popup is async.

With two popups, you would get

Previously:
SHOW A
HIDE A
SHOW B
NOTIFY SELECTION A
HIDE B
NOTIFY SELECTION B

When notifying the selection of A in the above scenario, it was
clearing the frame pointer (and actually updating the wrong popup).
Then when hiding the second popup, it's reference was actually gone
already.

Fix this by making the call to hide sync instead of async.

BUG=416354

Review URL: https://codereview.chromium.org/603303002

Cr-Commit-Position: refs/heads/master@{#296826}
parent 5da932e6
...@@ -2171,12 +2171,14 @@ public class ContentViewCore ...@@ -2171,12 +2171,14 @@ public class ContentViewCore
return; return;
} }
hidePopupsAndClearSelection();
assert mNativeSelectPopupSourceFrame == 0 : "Zombie popup did not clear the frame source";
assert items.length == enabled.length; assert items.length == enabled.length;
List<SelectPopupItem> popupItems = new ArrayList<SelectPopupItem>(); List<SelectPopupItem> popupItems = new ArrayList<SelectPopupItem>();
for (int i = 0; i < items.length; i++) { for (int i = 0; i < items.length; i++) {
popupItems.add(new SelectPopupItem(items[i], enabled[i])); popupItems.add(new SelectPopupItem(items[i], enabled[i]));
} }
hidePopupsAndClearSelection();
if (DeviceFormFactor.isTablet(mContext) && !multiple) { if (DeviceFormFactor.isTablet(mContext) && !multiple) {
mSelectPopup = new SelectPopupDropdown(this, popupItems, bounds, selectedIndices); mSelectPopup = new SelectPopupDropdown(this, popupItems, bounds, selectedIndices);
} else { } else {
......
...@@ -29,11 +29,12 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -29,11 +29,12 @@ public class SelectPopupDialog implements SelectPopup {
}; };
// The dialog hosting the popup list view. // The dialog hosting the popup list view.
private AlertDialog mListBoxPopup = null; private final AlertDialog mListBoxPopup;
private final ContentViewCore mContentViewCore; private final ContentViewCore mContentViewCore;
private final Context mContext; private final Context mContext;
private boolean mSelectionNotified;
public SelectPopupDialog(ContentViewCore contentViewCore, List<SelectPopupItem> items, public SelectPopupDialog(ContentViewCore contentViewCore, List<SelectPopupItem> items,
boolean multiple, int[] selected) { boolean multiple, int[] selected) {
mContentViewCore = contentViewCore; mContentViewCore = contentViewCore;
...@@ -50,14 +51,14 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -50,14 +51,14 @@ public class SelectPopupDialog implements SelectPopup {
b.setPositiveButton(android.R.string.ok, new DialogInterface.OnClickListener() { b.setPositiveButton(android.R.string.ok, new DialogInterface.OnClickListener() {
@Override @Override
public void onClick(DialogInterface dialog, int which) { public void onClick(DialogInterface dialog, int which) {
mContentViewCore.selectPopupMenuItems(getSelectedIndices(listView)); notifySelection(getSelectedIndices(listView));
} }
}); });
b.setNegativeButton(android.R.string.cancel, b.setNegativeButton(android.R.string.cancel,
new DialogInterface.OnClickListener() { new DialogInterface.OnClickListener() {
@Override @Override
public void onClick(DialogInterface dialog, int which) { public void onClick(DialogInterface dialog, int which) {
mContentViewCore.selectPopupMenuItems(null); notifySelection(null);
} }
}); });
} }
...@@ -78,7 +79,7 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -78,7 +79,7 @@ public class SelectPopupDialog implements SelectPopup {
@Override @Override
public void onItemClick(AdapterView<?> parent, View v, public void onItemClick(AdapterView<?> parent, View v,
int position, long id) { int position, long id) {
mContentViewCore.selectPopupMenuItems(getSelectedIndices(listView)); notifySelection(getSelectedIndices(listView));
mListBoxPopup.dismiss(); mListBoxPopup.dismiss();
} }
}); });
...@@ -90,7 +91,7 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -90,7 +91,7 @@ public class SelectPopupDialog implements SelectPopup {
mListBoxPopup.setOnCancelListener(new DialogInterface.OnCancelListener() { mListBoxPopup.setOnCancelListener(new DialogInterface.OnCancelListener() {
@Override @Override
public void onCancel(DialogInterface dialog) { public void onCancel(DialogInterface dialog) {
mContentViewCore.selectPopupMenuItems(null); notifySelection(null);
} }
}); });
} }
...@@ -104,7 +105,7 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -104,7 +105,7 @@ public class SelectPopupDialog implements SelectPopup {
return resourceId; return resourceId;
} }
private int[] getSelectedIndices(ListView listView) { private static int[] getSelectedIndices(ListView listView) {
SparseBooleanArray sparseArray = listView.getCheckedItemPositions(); SparseBooleanArray sparseArray = listView.getCheckedItemPositions();
int selectedCount = 0; int selectedCount = 0;
for (int i = 0; i < sparseArray.size(); ++i) { for (int i = 0; i < sparseArray.size(); ++i) {
...@@ -121,6 +122,12 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -121,6 +122,12 @@ public class SelectPopupDialog implements SelectPopup {
return indices; return indices;
} }
private void notifySelection(int[] indicies) {
if (mSelectionNotified) return;
mContentViewCore.selectPopupMenuItems(indicies);
mSelectionNotified = true;
}
@Override @Override
public void show() { public void show() {
mListBoxPopup.show(); mListBoxPopup.show();
...@@ -129,5 +136,6 @@ public class SelectPopupDialog implements SelectPopup { ...@@ -129,5 +136,6 @@ public class SelectPopupDialog implements SelectPopup {
@Override @Override
public void hide() { public void hide() {
mListBoxPopup.cancel(); mListBoxPopup.cancel();
notifySelection(null);
} }
} }
...@@ -25,10 +25,10 @@ public class SelectPopupDropdown implements SelectPopup { ...@@ -25,10 +25,10 @@ public class SelectPopupDropdown implements SelectPopup {
private final ContentViewCore mContentViewCore; private final ContentViewCore mContentViewCore;
private final Context mContext; private final Context mContext;
private final DropdownPopupWindow mDropdownPopupWindow;
private DropdownPopupWindow mDropdownPopupWindow;
private int mInitialSelection = -1; private int mInitialSelection = -1;
private boolean mAlreadySelectedItems = false; private boolean mSelectionNotified;
public SelectPopupDropdown(ContentViewCore contentViewCore, List<SelectPopupItem> items, public SelectPopupDropdown(ContentViewCore contentViewCore, List<SelectPopupItem> items,
Rect bounds, int[] selected) { Rect bounds, int[] selected) {
...@@ -39,9 +39,7 @@ public class SelectPopupDropdown implements SelectPopup { ...@@ -39,9 +39,7 @@ public class SelectPopupDropdown implements SelectPopup {
mDropdownPopupWindow.setOnItemClickListener(new AdapterView.OnItemClickListener() { mDropdownPopupWindow.setOnItemClickListener(new AdapterView.OnItemClickListener() {
@Override @Override
public void onItemClick(AdapterView<?> parent, View view, int position, long id) { public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
int[] selectedIndices = {position}; notifySelection(new int[] {position});
mContentViewCore.selectPopupMenuItems(selectedIndices);
mAlreadySelectedItems = true;
hide(); hide();
} }
}); });
...@@ -64,13 +62,17 @@ public class SelectPopupDropdown implements SelectPopup { ...@@ -64,13 +62,17 @@ public class SelectPopupDropdown implements SelectPopup {
new PopupWindow.OnDismissListener() { new PopupWindow.OnDismissListener() {
@Override @Override
public void onDismiss() { public void onDismiss() {
if (!mAlreadySelectedItems) { notifySelection(null);
mContentViewCore.selectPopupMenuItems(null);
}
} }
}); });
} }
private void notifySelection(int[] indicies) {
if (mSelectionNotified) return;
mContentViewCore.selectPopupMenuItems(indicies);
mSelectionNotified = true;
}
@Override @Override
public void show() { public void show() {
mDropdownPopupWindow.show(); mDropdownPopupWindow.show();
...@@ -82,5 +84,6 @@ public class SelectPopupDropdown implements SelectPopup { ...@@ -82,5 +84,6 @@ public class SelectPopupDropdown implements SelectPopup {
@Override @Override
public void hide() { public void hide() {
mDropdownPopupWindow.dismiss(); mDropdownPopupWindow.dismiss();
notifySelection(null);
} }
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment