Commit 43ceb11f authored by changwan's avatar changwan Committed by Commit bot

Cache proxy view return value to avoid deadlock

If WebView has an active IME but the Android app calls
InputMethodManager#hideSoftInputFromWindow() on a non-UI thread,
then a deadlock may happen.

One example case is when JavaScript triggers it through JavascriptInterface
(therefore, on JavaBridge thread.)

The deadlock scenario is as follows:

1) InputMethodManager#hideSoftFromWindow() calls
   ThreadedInputConnectionProxyView#getWindowToken() on JavaBridge thread
   while holding InputMethodManager#mH.
   Then getWindowToken() waits for UI thread to become available.
2) At almost same time, InputMethodManager#restartInput() was waiting for
   InputMethodManager#mH on UI thread

This deadlock can be avoided by caching return values as atomic objects.

Alternative approach I've tried: check the current thread and block
it only when it's IME thread - this may still leave room for deadlock
between IME thread and UI thread.

BUG=630937

Review-Url: https://codereview.chromium.org/2175263002
Cr-Commit-Position: refs/heads/master@{#408925}
parent da4e6a83
...@@ -1522,6 +1522,7 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Screen ...@@ -1522,6 +1522,7 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Screen
GamepadList.onAttachedToWindow(mContext); GamepadList.onAttachedToWindow(mContext);
mAccessibilityManager.addAccessibilityStateChangeListener(this); mAccessibilityManager.addAccessibilityStateChangeListener(this);
mSystemCaptioningBridge.addListener(this); mSystemCaptioningBridge.addListener(this);
mImeAdapter.onViewAttachedToWindow();
} }
/** /**
......
...@@ -29,6 +29,7 @@ public interface ChromiumBaseInputConnection extends InputConnection { ...@@ -29,6 +29,7 @@ public interface ChromiumBaseInputConnection extends InputConnection {
void onWindowFocusChanged(boolean gainFocus); void onWindowFocusChanged(boolean gainFocus);
void onViewFocusChanged(boolean gainFocus); void onViewFocusChanged(boolean gainFocus);
void onViewAttachedToWindow();
void onViewDetachedFromWindow(); void onViewDetachedFromWindow();
} }
......
...@@ -413,6 +413,15 @@ public class ImeAdapter { ...@@ -413,6 +413,15 @@ public class ImeAdapter {
} }
} }
/**
* Call this when view is detached from window
*/
public void onViewAttachedToWindow() {
if (mInputConnectionFactory != null) {
mInputConnectionFactory.onViewAttachedToWindow();
}
}
/** /**
* Call this when view is detached from window * Call this when view is detached from window
*/ */
......
...@@ -88,6 +88,9 @@ public class ReplicaInputConnection ...@@ -88,6 +88,9 @@ public class ReplicaInputConnection
@Override @Override
public void onViewFocusChanged(boolean gainFocus) {} public void onViewFocusChanged(boolean gainFocus) {}
@Override
public void onViewAttachedToWindow() {}
@Override @Override
public void onViewDetachedFromWindow() {} public void onViewDetachedFromWindow() {}
} }
......
...@@ -217,17 +217,26 @@ public class ThreadedInputConnectionFactory implements ChromiumBaseInputConnecti ...@@ -217,17 +217,26 @@ public class ThreadedInputConnectionFactory implements ChromiumBaseInputConnecti
public void onWindowFocusChanged(boolean gainFocus) { public void onWindowFocusChanged(boolean gainFocus) {
if (DEBUG_LOGS) Log.d(TAG, "onWindowFocusChanged: " + gainFocus); if (DEBUG_LOGS) Log.d(TAG, "onWindowFocusChanged: " + gainFocus);
if (!gainFocus && mCheckInvalidator != null) mCheckInvalidator.invalidate(); if (!gainFocus && mCheckInvalidator != null) mCheckInvalidator.invalidate();
if (mProxyView != null) mProxyView.onOriginalViewWindowFocusChanged(gainFocus);
} }
@Override @Override
public void onViewFocusChanged(boolean gainFocus) { public void onViewFocusChanged(boolean gainFocus) {
if (DEBUG_LOGS) Log.d(TAG, "onViewFocusChanged: " + gainFocus); if (DEBUG_LOGS) Log.d(TAG, "onViewFocusChanged: " + gainFocus);
if (!gainFocus && mCheckInvalidator != null) mCheckInvalidator.invalidate(); if (!gainFocus && mCheckInvalidator != null) mCheckInvalidator.invalidate();
if (mProxyView != null) mProxyView.onOriginalViewFocusChanged(gainFocus);
}
@Override
public void onViewAttachedToWindow() {
if (DEBUG_LOGS) Log.d(TAG, "onViewAttachedToWindow");
if (mProxyView != null) mProxyView.onOriginalViewAttachedToWindow();
} }
@Override @Override
public void onViewDetachedFromWindow() { public void onViewDetachedFromWindow() {
if (DEBUG_LOGS) Log.d(TAG, "onViewDetachedFromWindow"); if (DEBUG_LOGS) Log.d(TAG, "onViewDetachedFromWindow");
if (mCheckInvalidator != null) mCheckInvalidator.invalidate(); if (mCheckInvalidator != null) mCheckInvalidator.invalidate();
if (mProxyView != null) mProxyView.onOriginalViewDetachedFromWindow();
} }
} }
\ No newline at end of file
...@@ -15,6 +15,8 @@ import org.chromium.base.Log; ...@@ -15,6 +15,8 @@ import org.chromium.base.Log;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
/** /**
* This is a fake View that is only exposed to InputMethodManager. * This is a fake View that is only exposed to InputMethodManager.
...@@ -23,23 +25,58 @@ public class ThreadedInputConnectionProxyView extends View { ...@@ -23,23 +25,58 @@ public class ThreadedInputConnectionProxyView extends View {
private static final String TAG = "cr_Ime"; private static final String TAG = "cr_Ime";
private static final boolean DEBUG_LOGS = false; private static final boolean DEBUG_LOGS = false;
private final Handler mHandler; private final Handler mImeThreadHandler;
private final View mContainerView; private final View mContainerView;
private final AtomicBoolean mFocused = new AtomicBoolean();
private final AtomicBoolean mWindowFocused = new AtomicBoolean();
private final AtomicReference<IBinder> mWindowToken = new AtomicReference<>();
private final AtomicReference<View> mRootView = new AtomicReference<>();
ThreadedInputConnectionProxyView(Context context, Handler handler, View containerView) { ThreadedInputConnectionProxyView(
Context context, Handler imeThreadHandler, View containerView) {
super(context); super(context);
mHandler = handler; mImeThreadHandler = imeThreadHandler;
mContainerView = containerView; mContainerView = containerView;
setFocusable(true); setFocusable(true);
setFocusableInTouchMode(true); setFocusableInTouchMode(true);
setVisibility(View.VISIBLE); setVisibility(View.VISIBLE);
if (DEBUG_LOGS) Log.w(TAG, "constructor"); if (DEBUG_LOGS) Log.w(TAG, "constructor");
mFocused.set(mContainerView.hasFocus());
mWindowFocused.set(mContainerView.hasWindowFocus());
mWindowToken.set(mContainerView.getWindowToken());
mRootView.set(mContainerView.getRootView());
}
public void onOriginalViewFocusChanged(boolean gainFocus) {
mFocused.set(gainFocus);
}
public void onOriginalViewWindowFocusChanged(boolean gainFocus) {
mWindowFocused.set(gainFocus);
}
public void onOriginalViewAttachedToWindow() {
mWindowToken.set(mContainerView.getWindowToken());
// Note: this is an approximation of the real behavior.
// Real root view may change upon addView / removeView, but this is good
// enough for IME purpose.
mRootView.set(mContainerView.getRootView());
}
public void onOriginalViewDetachedFromWindow() {
mWindowToken.set(null);
// Note: we are not asking mContainerView.getRootView() here. We cannot get the correct
// root view here as ViewRootImpl's mParent is set to null *after* this call.
// In vanilla Android, getRootView() is never called when window is detaching or detached
// anyways.
mRootView.set(null);
} }
@Override @Override
public Handler getHandler() { public Handler getHandler() {
if (DEBUG_LOGS) Log.w(TAG, "getHandler"); if (DEBUG_LOGS) Log.w(TAG, "getHandler");
return mHandler; return mImeThreadHandler;
} }
@Override @Override
...@@ -59,70 +96,35 @@ public class ThreadedInputConnectionProxyView extends View { ...@@ -59,70 +96,35 @@ public class ThreadedInputConnectionProxyView extends View {
}); });
} }
@Override
public boolean hasFocus() {
if (DEBUG_LOGS) Log.w(TAG, "hasFocus");
return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() {
@Override
public Boolean call() throws Exception {
return mContainerView.hasFocus();
}
});
}
@Override @Override
public boolean hasWindowFocus() { public boolean hasWindowFocus() {
if (DEBUG_LOGS) Log.w(TAG, "hasWindowFocus"); if (DEBUG_LOGS) Log.w(TAG, "hasWindowFocus");
return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { return mWindowFocused.get();
@Override
public Boolean call() throws Exception {
return mContainerView.hasWindowFocus();
}
});
} }
@Override @Override
public View getRootView() { public View getRootView() {
if (DEBUG_LOGS) Log.w(TAG, "getRootView"); if (DEBUG_LOGS) Log.w(TAG, "getRootView");
return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<View>() { return mRootView.get();
@Override
public View call() throws Exception {
return mContainerView.getRootView();
}
});
} }
@Override @Override
public boolean onCheckIsTextEditor() { public boolean onCheckIsTextEditor() {
if (DEBUG_LOGS) Log.w(TAG, "onCheckIsTextEditor"); if (DEBUG_LOGS) Log.w(TAG, "onCheckIsTextEditor");
return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { // We do not allow Android apps to override WebView#onCheckIsTextEditor() for now.
@Override return true;
public Boolean call() throws Exception {
return mContainerView.onCheckIsTextEditor();
}
});
} }
@Override @Override
public boolean isFocused() { public boolean isFocused() {
if (DEBUG_LOGS) Log.w(TAG, "isFocused"); if (DEBUG_LOGS) Log.w(TAG, "isFocused");
return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() { return mFocused.get();
@Override
public Boolean call() throws Exception {
return mContainerView.isFocused();
}
});
} }
@Override @Override
public IBinder getWindowToken() { public IBinder getWindowToken() {
if (DEBUG_LOGS) Log.w(TAG, "getWindowToken"); if (DEBUG_LOGS) Log.w(TAG, "getWindowToken");
return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<IBinder>() { return mWindowToken.get();
@Override
public IBinder call() throws Exception {
return mContainerView.getWindowToken();
}
});
} }
@Override @Override
......
...@@ -1693,6 +1693,11 @@ public class ImeTest extends ContentShellTestBase { ...@@ -1693,6 +1693,11 @@ public class ImeTest extends ContentShellTestBase {
mFactory.onViewFocusChanged(gainFocus); mFactory.onViewFocusChanged(gainFocus);
} }
@Override
public void onViewAttachedToWindow() {
mFactory.onViewAttachedToWindow();
}
@Override @Override
public void onViewDetachedFromWindow() { public void onViewDetachedFromWindow() {
mFactory.onViewDetachedFromWindow(); mFactory.onViewDetachedFromWindow();
......
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