From 8a599aa552b74a5d6d0767a2a0dce79578d1796d Mon Sep 17 00:00:00 2001 From: Jim Chen Date: Tue, 6 Nov 2018 00:12:06 -0500 Subject: [PATCH] Bug 1499429 - 3. Transfer to new GeckoEditableParent during session transfer; r=esawin During a session transfer, update existing GeckoEditableChild instances in the parent and child processes to use the new GeckoEditableParent instance that corresponds to the new session. If the GeckoEditableChild has focus, take additional steps to make sure the GeckoEditableParent receives current input context and focus information. Differential Revision: https://phabricator.services.mozilla.com/D8996 --- .../mozilla/gecko/IGeckoEditableParent.aidl | 4 +- .../org/mozilla/gecko/GeckoEditableChild.java | 13 ++- .../org/mozilla/geckoview/GeckoEditable.java | 13 ++- widget/android/GeckoEditableSupport.cpp | 90 +++++++++++++++---- widget/android/GeckoEditableSupport.h | 20 +++++ widget/android/nsAppShell.cpp | 42 ++------- widget/android/nsWindow.cpp | 13 ++- 7 files changed, 131 insertions(+), 64 deletions(-) diff --git a/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/IGeckoEditableParent.aidl b/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/IGeckoEditableParent.aidl index 2ed90d806612..60ce9c3e0114 100644 --- a/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/IGeckoEditableParent.aidl +++ b/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/IGeckoEditableParent.aidl @@ -18,8 +18,8 @@ interface IGeckoEditableParent { void notifyIME(IGeckoEditableChild child, int type); // Notify a change in editor state or type. - void notifyIMEContext(int state, String typeHint, String modeHint, String actionHint, - int flags); + void notifyIMEContext(IBinder token, int state, String typeHint, String modeHint, + String actionHint, int flags); // Notify a change in editor selection. void onSelectionChange(IBinder token, int start, int end); diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditableChild.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditableChild.java index 450dd0cf0cfa..bb51ae008066 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditableChild.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditableChild.java @@ -70,17 +70,16 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl } } - private final IGeckoEditableParent mEditableParent; private final IGeckoEditableChild mEditableChild; private final boolean mIsDefault; + private IGeckoEditableParent mEditableParent; private int mCurrentTextLength; // Used by Gecko thread @WrapForJNI(calledFrom = "gecko") private GeckoEditableChild(final IGeckoEditableParent editableParent, final boolean isDefault) { mIsDefault = isDefault; - mEditableParent = editableParent; final IBinder binder = editableParent.asBinder(); if (binder.queryLocalInterface(IGeckoEditableParent.class.getName()) != null) { @@ -91,6 +90,13 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl mEditableChild = new RemoteChild(); } + setParent(editableParent); + } + + @WrapForJNI(calledFrom = "gecko") + private void setParent(final IGeckoEditableParent editableParent) { + mEditableParent = editableParent; + if (mIsDefault) { // Tell the parent we're the default child. try { @@ -175,7 +181,8 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl } try { - mEditableParent.notifyIMEContext(state, typeHint, modeHint, actionHint, flags); + mEditableParent.notifyIMEContext(mEditableChild.asBinder(), state, typeHint, + modeHint, actionHint, flags); } catch (final RemoteException e) { Log.e(LOGTAG, "Remote call failed", e); } diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java index bcdb4a36f904..89916e868879 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java @@ -1292,7 +1292,7 @@ import android.view.inputmethod.EditorInfo; } @Override // IGeckoEditableParent - public void notifyIMEContext(final int state, final String typeHint, + public void notifyIMEContext(final IBinder token, final int state, final String typeHint, final String modeHint, final String actionHint, final int flags) { // On Gecko or binder thread. @@ -1304,9 +1304,14 @@ import android.view.inputmethod.EditorInfo; "\", 0x" + Integer.toHexString(flags) + ")"); } - // Don't check token for notifyIMEContext, because the calls all come - // from the parent process. - ThreadUtils.assertOnGeckoThread(); + // Regular notifyIMEContext calls all come from the parent process (with the default child), + // so always allow calls from there. We can get additional notifyIMEContext calls during + // a session transfer; calls in those cases can come from child processes, and we must + // perform a token check in that situation. + if (token != mDefaultChild.asBinder() && + !binderCheckToken(token, /* allowNull */ false)) { + return; + } mIcPostHandler.post(new Runnable() { @Override diff --git a/widget/android/GeckoEditableSupport.cpp b/widget/android/GeckoEditableSupport.cpp index c275daeba3e1..0caf4b20af2d 100644 --- a/widget/android/GeckoEditableSupport.cpp +++ b/widget/android/GeckoEditableSupport.cpp @@ -16,6 +16,7 @@ #include "mozilla/TextComposition.h" #include "mozilla/TextEventDispatcherListener.h" #include "mozilla/TextEvents.h" +#include "mozilla/dom/TabChild.h" #include #include @@ -1425,27 +1426,34 @@ GeckoEditableSupport::SetInputContext(const InputContext& aContext, return; } - const bool inPrivateBrowsing = mInputContext.mInPrivateBrowsing; + // Post an event to keep calls in order relative to NotifyIME. + nsAppShell::PostEvent([this, self = RefPtr(this), + context = mInputContext, action = aAction] { + nsCOMPtr widget = GetWidget(); + + if (!widget || widget->Destroyed()) { + return; + } + NotifyIMEContext(context, action); + }); +} + +void +GeckoEditableSupport::NotifyIMEContext(const InputContext& aContext, + const InputContextAction& aAction) +{ + const bool inPrivateBrowsing = aContext.mInPrivateBrowsing; const bool isUserAction = aAction.IsHandlingUserInput() || aContext.mHasHandledUserInput; const int32_t flags = (inPrivateBrowsing ? EditableListener::IME_FLAG_PRIVATE_BROWSING : 0) | (isUserAction ? EditableListener::IME_FLAG_USER_ACTION : 0); - // Post an event to keep calls in order relative to NotifyIME. - nsAppShell::PostEvent([this, self = RefPtr(this), - flags, context = mInputContext] { - nsCOMPtr widget = GetWidget(); - - if (!widget || widget->Destroyed()) { - return; - } - mEditable->NotifyIMEContext(context.mIMEState.mEnabled, - context.mHTMLInputType, - context.mHTMLInputInputmode, - context.mActionHint, - flags); - }); + mEditable->NotifyIMEContext(aContext.mIMEState.mEnabled, + aContext.mHTMLInputType, + aContext.mHTMLInputInputmode, + aContext.mActionHint, + flags); } InputContext @@ -1456,5 +1464,57 @@ GeckoEditableSupport::GetInputContext() return context; } +void +GeckoEditableSupport::SetOnTabChild(dom::TabChild* aTabChild) +{ + MOZ_ASSERT(!XRE_IsParentProcess()); + NS_ENSURE_TRUE_VOID(aTabChild); + + const dom::ContentChild* const contentChild = + dom::ContentChild::GetSingleton(); + RefPtr widget(aTabChild->WebWidget()); + NS_ENSURE_TRUE_VOID(contentChild && widget); + + // Get the content/tab ID in order to get the correct + // IGeckoEditableParent object, which GeckoEditableChild uses to + // communicate with the parent process. + const uint64_t contentId = contentChild->GetID(); + 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 listener = + widget->GetNativeTextEventDispatcherListener(); + + if (!listener || listener.get() == + static_cast(widget)) { + // We need to set a new listener. + auto editableChild = java::GeckoEditableChild::New(editableParent, + /* default */ false); + RefPtr editableSupport = + new widget::GeckoEditableSupport(editableChild); + + // Tell PuppetWidget to use our listener for IME operations. + widget->SetNativeTextEventDispatcherListener(editableSupport); + return; + } + + // We need to update the existing listener to use the new parent. + + // 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 dummy = + new widget::GeckoEditableSupport(/* child */ nullptr); + NS_ENSURE_TRUE_VOID(*reinterpret_cast(listener.get()) == + *reinterpret_cast(dummy.get())); + + static_cast( + listener.get())->TransferParent(editableParent); +} + } // namespace widget } // namespace mozilla diff --git a/widget/android/GeckoEditableSupport.h b/widget/android/GeckoEditableSupport.h index 763991071f09..2b14911c1e34 100644 --- a/widget/android/GeckoEditableSupport.h +++ b/widget/android/GeckoEditableSupport.h @@ -21,6 +21,10 @@ namespace mozilla { class TextComposition; +namespace dom { +class TabChild; +} + namespace widget { class GeckoEditableSupport final @@ -128,6 +132,8 @@ class GeckoEditableSupport final void AsyncNotifyIME(int32_t aNotification); void UpdateCompositionRects(); bool DoReplaceText(int32_t aStart, int32_t aEnd, jni::String::Param aText); + void NotifyIMEContext(const InputContext& aContext, + const InputContextAction& aAction); public: template @@ -162,6 +168,8 @@ public: std::move(aCall))); } + static void SetOnTabChild(dom::TabChild* aTabChild); + // Constructor for main process GeckoEditableChild. GeckoEditableSupport(nsWindow::NativePtr* aPtr, nsWindow* aWindow, @@ -215,6 +223,18 @@ 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 aDisposer) { RefPtr self(this); diff --git a/widget/android/nsAppShell.cpp b/widget/android/nsAppShell.cpp index c9ea4a3027f9..c214fe88b781 100644 --- a/widget/android/nsAppShell.cpp +++ b/widget/android/nsAppShell.cpp @@ -22,7 +22,6 @@ #include "nsIDOMWakeLockListener.h" #include "nsIPowerManagerService.h" #include "nsISpeculativeConnect.h" -#include "nsITabChild.h" #include "nsIURIFixup.h" #include "nsCategoryManagerUtils.h" #include "nsCDefaultURIFixup.h" @@ -553,6 +552,7 @@ nsAppShell::Init() obsServ->AddObserver(this, "chrome-document-loaded", false); } else { obsServ->AddObserver(this, "content-document-global-created", false); + obsServ->AddObserver(this, "geckoview-content-global-transferred", false); } } @@ -678,39 +678,15 @@ nsAppShell::Observe(nsISupports* aSubject, nsPIDOMWindowOuter::From(domWindow)); NS_ENSURE_TRUE(domWidget, NS_OK); - dom::ContentChild* contentChild = dom::ContentChild::GetSingleton(); - dom::TabChild* tabChild = domWidget->GetOwningTabChild(); - RefPtr widget(tabChild->WebWidget()); - NS_ENSURE_TRUE(contentChild && tabChild && widget, NS_OK); + widget::GeckoEditableSupport::SetOnTabChild( + domWidget->GetOwningTabChild()); - widget::TextEventDispatcherListener* listener = - widget->GetNativeTextEventDispatcherListener(); - if (listener && listener != - static_cast(widget)) { - // We already set a listener before. - return NS_OK; - } - - // Get the content/tab ID in order to get the correct - // IGeckoEditableParent object, which GeckoEditableChild uses to - // communicate with the parent process. - const uint64_t contentId = contentChild->GetID(); - const uint64_t tabId = tabChild->GetTabId(); - NS_ENSURE_TRUE(contentId && tabId, NS_OK); - - auto editableParent = java::GeckoServiceChildProcess::GetEditableParent( - contentId, tabId); - NS_ENSURE_TRUE(editableParent, NS_OK); - - auto editableChild = java::GeckoEditableChild::New(editableParent, - /* default */ false); - NS_ENSURE_TRUE(editableChild, NS_OK); - - RefPtr editableSupport = - new widget::GeckoEditableSupport(editableChild); - - // Tell PuppetWidget to use our listener for IME operations. - widget->SetNativeTextEventDispatcherListener(editableSupport); + } else if (!strcmp(aTopic, "geckoview-content-global-transferred")) { + // We're transferring to a new GeckoEditableParent, so notify the + // existing GeckoEditableChild instance associated with the docshell. + nsCOMPtr docShell = do_QueryInterface(aSubject); + widget::GeckoEditableSupport::SetOnTabChild( + dom::TabChild::GetFrom(docShell)); } if (removeObserver) { diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index ba1f6816e5f0..a97589a5661b 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -1363,15 +1363,14 @@ void nsWindow::GeckoViewSupport::AttachEditable(const GeckoSession::Window::LocalRef& inst, jni::Object::Param aEditableParent) { - auto editableChild = java::GeckoEditableChild::New(aEditableParent, - /* default */ true); - - if (window.mEditableSupport) { - window.mEditableSupport.Detach( - window.mEditableSupport->GetJavaEditable()); + if (!window.mEditableSupport) { + auto editableChild = java::GeckoEditableChild::New(aEditableParent, + /* default */ true); + window.mEditableSupport.Attach(editableChild, &window, editableChild); + } else { + window.mEditableSupport->TransferParent(aEditableParent); } - window.mEditableSupport.Attach(editableChild, &window, editableChild); window.mEditableParent = aEditableParent; }