Bug 1499429 - 5. Make setting editable parent async; r=esawin

Currently we make a sync call from the child process to the parent
process to retrieve the IGeckoEditableParent instance. However, that can
lead to deadlocks when a11y code makes parent-to-child async calls at
the same time. This patch makes the call async to avoid the deadlock.

Differential Revision: https://phabricator.services.mozilla.com/D10663
This commit is contained in:
Jim Chen 2018-11-06 00:12:07 -05:00
Родитель 7b5fb126e0
Коммит 12453d9645
8 изменённых файлов: 131 добавлений и 50 удалений

Просмотреть файл

@ -4,10 +4,15 @@
package org.mozilla.gecko;
import org.mozilla.gecko.IGeckoEditableParent;
import android.view.KeyEvent;
// Interface for GeckoEditable calls from parent to child
interface IGeckoEditableChild {
// Transfer this child to a new parent.
void transferParent(in IGeckoEditableParent parent);
// Process a key event.
void onKeyEvent(int action, int keyCode, int scanCode, int metaState,
int keyPressMetaState, long time, int domPrintableKeyValue,

Просмотреть файл

@ -4,8 +4,8 @@
package org.mozilla.gecko.process;
import org.mozilla.gecko.IGeckoEditableParent;
import org.mozilla.gecko.IGeckoEditableChild;
interface IProcessManager {
IGeckoEditableParent getEditableParent(long contentId, long tabId);
void getEditableParent(in IGeckoEditableChild child, long contentId, long tabId);
}

Просмотреть файл

@ -12,6 +12,7 @@ import org.mozilla.gecko.util.ThreadUtils;
import android.graphics.RectF;
import android.os.IBinder;
import android.os.RemoteException;
import android.support.annotation.Nullable;
import android.util.Log;
import android.view.KeyEvent;
@ -29,6 +30,11 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
private static final int NOTIFY_IME_TO_CANCEL_COMPOSITION = 9;
private final class RemoteChild extends IGeckoEditableChild.Stub {
@Override // IGeckoEditableChild
public void transferParent(final IGeckoEditableParent editableParent) {
GeckoEditableChild.this.transferParent(editableParent);
}
@Override // IGeckoEditableChild
public void onKeyEvent(int action, int keyCode, int scanCode, int metaState,
int keyPressMetaState, long time, int domPrintableKeyValue,
@ -77,12 +83,13 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
private int mCurrentTextLength; // Used by Gecko thread
@WrapForJNI(calledFrom = "gecko")
private GeckoEditableChild(final IGeckoEditableParent editableParent,
private GeckoEditableChild(@Nullable final IGeckoEditableParent editableParent,
final boolean isDefault) {
mIsDefault = isDefault;
final IBinder binder = editableParent.asBinder();
if (binder.queryLocalInterface(IGeckoEditableParent.class.getName()) != null) {
if (editableParent != null &&
editableParent.asBinder().queryLocalInterface(
IGeckoEditableParent.class.getName()) != null) {
// IGeckoEditableParent is local; i.e. we're in the main process.
mEditableChild = this;
} else {
@ -90,7 +97,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
mEditableChild = new RemoteChild();
}
setParent(editableParent);
if (editableParent != null) {
setParent(editableParent);
}
}
@WrapForJNI(calledFrom = "gecko")
@ -107,6 +116,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
}
}
@WrapForJNI(dispatchTo = "proxy") @Override // IGeckoEditableChild
public native void transferParent(IGeckoEditableParent editableParent);
@WrapForJNI(dispatchTo = "proxy") @Override // IGeckoEditableChild
public native void onKeyEvent(int action, int keyCode, int scanCode, int metaState,
int keyPressMetaState, long time, int domPrintableKeyValue,
@ -141,10 +153,22 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
throw new UnsupportedOperationException();
}
@WrapForJNI(calledFrom = "gecko")
private boolean hasEditableParent() {
if (mEditableParent != null) {
return true;
}
Log.w(LOGTAG, "No editable parent");
return false;
}
@Override // IInterface
public IBinder asBinder() {
// Return the GeckoEditableParent's binder as our binder for comparison purposes.
return mEditableParent.asBinder();
// Return the GeckoEditableParent's binder as fallback for comparison purposes.
return mEditableChild != this
? mEditableChild.asBinder()
: hasEditableParent()
? mEditableParent.asBinder() : null;
}
@WrapForJNI(calledFrom = "gecko")
@ -153,6 +177,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
ThreadUtils.assertOnGeckoThread();
Log.d(LOGTAG, "notifyIME(" + type + ")");
}
if (!hasEditableParent()){
return;
}
if (type == NOTIFY_IME_TO_CANCEL_COMPOSITION) {
// Composition should have been canceled on the parent side through text
// update notifications. We cannot verify that here because we don't
@ -179,6 +206,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
typeHint + "\", \"" + modeHint + "\", \"" + actionHint +
"\", 0x" + Integer.toHexString(flags) + ")");
}
if (!hasEditableParent()){
return;
}
try {
mEditableParent.notifyIMEContext(mEditableChild.asBinder(), state, typeHint,
@ -194,6 +224,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
ThreadUtils.assertOnGeckoThread();
Log.d(LOGTAG, "onSelectionChange(" + start + ", " + end + ")");
}
if (!hasEditableParent()){
return;
}
final int currentLength = mCurrentTextLength;
if (start < 0 || start > currentLength || end < 0 || end > currentLength) {
@ -214,6 +247,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
Log.d(LOGTAG, "onTextChange(" + text + ", " + start + ", " +
unboundedOldEnd + ", " + unboundedNewEnd + ")");
}
if (!hasEditableParent()){
return;
}
if (start < 0 || start > unboundedOldEnd) {
Log.e(LOGTAG, "invalid text notification range: " +
@ -250,6 +286,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
.append("repeatCount=").append(event.getRepeatCount()).append(")");
Log.d(LOGTAG, sb.toString());
}
if (!hasEditableParent()){
return;
}
try {
mEditableParent.onDefaultKeyEvent(mEditableChild.asBinder(), event);
@ -265,6 +304,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
ThreadUtils.assertOnGeckoThread();
Log.d(LOGTAG, "updateCompositionRects(rects.length = " + rects.length + ")");
}
if (!hasEditableParent()){
return;
}
try {
mEditableParent.updateCompositionRects(mEditableChild.asBinder(), rects);

Просмотреть файл

@ -6,6 +6,7 @@ package org.mozilla.gecko.process;
import org.mozilla.gecko.GeckoAppShell;
import org.mozilla.gecko.GeckoThread;
import org.mozilla.gecko.IGeckoEditableChild;
import org.mozilla.gecko.IGeckoEditableParent;
import org.mozilla.gecko.annotation.WrapForJNI;
import org.mozilla.gecko.util.ThreadUtils;
@ -31,13 +32,24 @@ public final class GeckoProcessManager extends IProcessManager.Stub {
return INSTANCE;
}
@WrapForJNI(stubName = "GetEditableParent")
private static native IGeckoEditableParent nativeGetEditableParent(long contentId,
long tabId);
@WrapForJNI(calledFrom = "gecko")
private static void setEditableChildParent(final IGeckoEditableChild child,
final IGeckoEditableParent parent) {
try {
child.transferParent(parent);
} catch (final RemoteException e) {
Log.e(LOGTAG, "Cannot set parent", e);
}
}
@WrapForJNI(stubName = "GetEditableParent", dispatchTo = "gecko")
private static native void nativeGetEditableParent(IGeckoEditableChild child,
long contentId, long tabId);
@Override // IProcessManager
public IGeckoEditableParent getEditableParent(final long contentId, final long tabId) {
return nativeGetEditableParent(contentId, tabId);
public void getEditableParent(final IGeckoEditableChild child,
final long contentId, final long tabId) {
nativeGetEditableParent(child, contentId, tabId);
}
private static final class ChildConnection implements ServiceConnection,

Просмотреть файл

@ -7,6 +7,7 @@ package org.mozilla.gecko.process;
import org.mozilla.gecko.annotation.WrapForJNI;
import org.mozilla.gecko.GeckoAppShell;
import org.mozilla.gecko.IGeckoEditableChild;
import org.mozilla.gecko.IGeckoEditableParent;
import org.mozilla.gecko.mozglue.GeckoLoader;
import org.mozilla.gecko.GeckoThread;
@ -28,13 +29,13 @@ public class GeckoServiceChildProcess extends Service {
private static IProcessManager sProcessManager;
@WrapForJNI(calledFrom = "gecko")
private static IGeckoEditableParent getEditableParent(final long contentId,
final long tabId) {
private static void getEditableParent(final IGeckoEditableChild child,
final long contentId,
final long tabId) {
try {
return sProcessManager.getEditableParent(contentId, tabId);
sProcessManager.getEditableParent(child, contentId, tabId);
} catch (final RemoteException e) {
Log.e(LOGTAG, "Cannot get editable", e);
return null;
}
}

Просмотреть файл

@ -1384,7 +1384,7 @@ GeckoEditableSupport::OnRemovedFrom(TextEventDispatcher* aTextEventDispatcher)
{
mDispatcher = nullptr;
if (mIsRemote) {
if (mIsRemote && mEditable->HasEditableParent()) {
// When we're remote, detach every time.
OnDetach(NS_NewRunnableFunction("GeckoEditableSupport::OnRemovedFrom",
[editable = java::GeckoEditableChild::GlobalRef(mEditable)] {
@ -1464,6 +1464,25 @@ GeckoEditableSupport::GetInputContext()
return context;
}
void
GeckoEditableSupport::TransferParent(jni::Object::Param aEditableParent)
{
mEditable->SetParent(aEditableParent);
// If we are already focused, make sure the new parent has our token
// and focus information, so it can accept additional calls from us.
if (mIMEFocusCount > 0) {
mEditable->NotifyIME(EditableListener::NOTIFY_IME_OF_TOKEN);
NotifyIMEContext(mInputContext, InputContextAction());
mEditable->NotifyIME(EditableListener::NOTIFY_IME_OF_FOCUS);
}
if (mIsRemote && !mDispatcher) {
// Detach now if we were only attached temporarily.
OnRemovedFrom(/* dispatcher */ nullptr);
}
}
void
GeckoEditableSupport::SetOnTabChild(dom::TabChild* aTabChild)
{
@ -1482,23 +1501,27 @@ GeckoEditableSupport::SetOnTabChild(dom::TabChild* aTabChild)
const uint64_t tabId = aTabChild->GetTabId();
NS_ENSURE_TRUE_VOID(contentId && tabId);
auto editableParent = java::GeckoServiceChildProcess::GetEditableParent(
contentId, tabId);
NS_ENSURE_TRUE_VOID(editableParent);
RefPtr<widget::TextEventDispatcherListener> listener =
widget->GetNativeTextEventDispatcherListener();
if (!listener || listener.get() ==
static_cast<widget::TextEventDispatcherListener*>(widget)) {
// We need to set a new listener.
auto editableChild = java::GeckoEditableChild::New(editableParent,
/* default */ false);
const auto editableChild = java::GeckoEditableChild::New(
/* parent */ nullptr, /* default */ false);
RefPtr<widget::GeckoEditableSupport> editableSupport =
new widget::GeckoEditableSupport(editableChild);
// Tell PuppetWidget to use our listener for IME operations.
widget->SetNativeTextEventDispatcherListener(editableSupport);
// Temporarily attach so we can receive the initial editable parent.
AttachNative(editableChild, editableSupport);
editableSupport->mEditableAttached = true;
// Connect the new child to a parent that corresponds to the TabChild.
java::GeckoServiceChildProcess::GetEditableParent(
editableChild, contentId, tabId);
return;
}
@ -1507,13 +1530,22 @@ GeckoEditableSupport::SetOnTabChild(dom::TabChild* aTabChild)
// We expect the existing TextEventDispatcherListener to be a
// GeckoEditableSupport object, so we perform a sanity check to make
// sure, by comparing their respective vtable pointers.
RefPtr<widget::GeckoEditableSupport> dummy =
const RefPtr<widget::GeckoEditableSupport> dummy =
new widget::GeckoEditableSupport(/* child */ nullptr);
NS_ENSURE_TRUE_VOID(*reinterpret_cast<const uintptr_t*>(listener.get()) ==
*reinterpret_cast<const uintptr_t*>(dummy.get()));
static_cast<widget::GeckoEditableSupport*>(
listener.get())->TransferParent(editableParent);
const auto support =
static_cast<widget::GeckoEditableSupport*>(listener.get());
if (!support->mEditableAttached) {
// Temporarily attach so we can receive the initial editable parent.
AttachNative(support->GetJavaEditable(), support);
support->mEditableAttached = true;
}
// Transfer to a new parent that corresponds to the TabChild.
java::GeckoServiceChildProcess::GetEditableParent(
support->GetJavaEditable(), contentId, tabId);
}
} // namespace widget

Просмотреть файл

@ -223,18 +223,6 @@ public:
const java::GeckoEditableChild::Ref& GetJavaEditable() { return mEditable; }
void TransferParent(jni::Object::Param aEditableParent) {
mEditable->SetParent(aEditableParent);
// If we are already focused, make sure the new parent has our token
// and focus information, so it can accept additional calls from us.
if (mIMEFocusCount > 0) {
mEditable->NotifyIME(EditableListener::NOTIFY_IME_OF_TOKEN);
NotifyIMEContext(mInputContext, InputContextAction());
mEditable->NotifyIME(EditableListener::NOTIFY_IME_OF_FOCUS);
}
}
void OnDetach(already_AddRefed<Runnable> aDisposer)
{
RefPtr<GeckoEditableSupport> self(this);
@ -245,6 +233,9 @@ public:
});
}
// Transfer to a new parent.
void TransferParent(jni::Object::Param aEditableParent);
// Handle an Android KeyEvent.
void OnKeyEvent(int32_t aAction, int32_t aKeyCode, int32_t aScanCode,
int32_t aMetaState, int32_t aKeyPressMetaState,

Просмотреть файл

@ -42,18 +42,16 @@ class GeckoProcessManager final
}
public:
static jni::Object::LocalRef
GetEditableParent(int64_t aContentId, int64_t aTabId)
static void
GetEditableParent(jni::Object::Param aEditableChild,
int64_t aContentId, int64_t aTabId)
{
// On binder thread.
jni::Object::GlobalRef ret;
nsAppShell::SyncRunEvent([aContentId, aTabId, &ret] {
nsCOMPtr<nsIWidget> widget = GetWidget(aContentId, aTabId);
if (widget) {
ret = static_cast<nsWindow*>(widget.get())->GetEditableParent();
}
});
return ret;
nsCOMPtr<nsIWidget> widget = GetWidget(aContentId, aTabId);
if (widget && widget->GetNativeData(NS_NATIVE_WIDGET) == widget) {
java::GeckoProcessManager::SetEditableChildParent(
aEditableChild,
static_cast<nsWindow*>(widget.get())->GetEditableParent());
}
}
};