Commit b6ef18c6 authored by Sky Malice's avatar Sky Malice Committed by Commit Bot

[Touchless] No-op creating a new context menu dialog when one is

already shown.

There is a race condition in the logic creating context menus on
touchless. The TouchlessModelCoordinatorImpl is notified
asynchronously, and it is typically the thing that guards us from
creating a second context menu. This patch fixes this by having the
TouchlessContextMenuManager implement its own guarding logic, which
will work synchronously and not be vulnerable to any races.

This solution relies on the ModalDialogManager always notifying the
TouchlessContextMenuManager when the previous dialog was closed.
Otherwise we could get stuck in a state where the
TouchlessContextMenuManager thinks there's an open dialog when there
isn't actually one and the user is unable to open the context menu.

Bug: 982826
Change-Id: Ibf030b637c5d55122108731fa727976f967e168f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699210Reviewed-by: default avatarPavel Yatsuk <pavely@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676632}
parent f1054dc5
......@@ -13,7 +13,6 @@ import android.view.View.OnClickListener;
import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.native_page.ContextMenuManager;
import org.chromium.chrome.browser.native_page.ContextMenuManager.ContextMenuItemId;
import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
import org.chromium.chrome.browser.touchless.dialog.TouchlessDialogProperties;
import org.chromium.chrome.browser.touchless.dialog.TouchlessDialogProperties.ActionNames;
......@@ -63,6 +62,8 @@ public class TouchlessContextMenuManager extends ContextMenuManager {
private final ChromeActivity mActivity;
private final ModalDialogManager mDialogManager;
// This field should be set when showing a dialog, and nulled out when the dialog is closed. We
// can check if it is null to determine whether we're currently showing our dialog.
private PropertyModel mTouchlessMenuModel;
private ModalDialogManager mModalDialogManager;
......@@ -84,6 +85,11 @@ public class TouchlessContextMenuManager extends ContextMenuManager {
*/
public void showTouchlessContextMenu(
ModalDialogManager modalDialogManager, ContextMenuManager.Delegate delegate) {
// Don't create a new dialog if we're already showing one.
if (mTouchlessMenuModel != null) {
return;
}
ArrayList<PropertyModel> menuItems = new ArrayList<>();
for (@ContextMenuItemId int itemId = 0; itemId < ContextMenuItemId.NUM_ENTRIES; itemId++) {
if (!shouldShowItem(itemId, delegate)) continue;
......@@ -195,7 +201,9 @@ public class TouchlessContextMenuManager extends ContextMenuManager {
public void onClick(PropertyModel model, int buttonType) {}
@Override
public void onDismiss(PropertyModel model, int dismissalCause) {}
public void onDismiss(PropertyModel model, int dismissalCause) {
mTouchlessMenuModel = null;
}
})
.with(TouchlessDialogProperties.ACTION_NAMES, names)
.with(TouchlessDialogProperties.CANCEL_ACTION, (v) -> closeTouchlessContextMenu())
......
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