Commit ee89226f authored by changwan's avatar changwan Committed by Commit bot

Revert of Migrate IME state update flow (patchset #4 id:100001 of...

Revert of Migrate IME state update flow (patchset #4 id:100001 of https://codereview.chromium.org/2777223004/ )

Reason for revert:
This patchset caused a regression crbug.com/709349: 'cut' option disappeared from webview selection pop up.

Original issue's description:
> Migrate IME state update flow
>
> Refactored the flow for IME state update so it bypasses CVCImpl
> and go straight from RWHVA -> ImeAdapter native -> Java layer.
>
> Other related changes are:
>
> ImeAdapter provides EventObserver to reduce the dependency on CVC,
> based on the suggestion made in https://goo.gl/pdtQCl. It is used
> by an embedder (Chrome) and CVC to deal with IME notification.
> This replaces IME state update done through CVC. Only the necessary
> info (node editability, password attribute) are passed.
>
> ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
> events. But it is used by ContentViewCore only, and there is no
> clear benefit of having the interface. It was removed for
> simplification. All the stuff can be (and are now) handled inside
> ImeAdapter.
>
> BUG=662908,626765,620172
>
> Review-Url: https://codereview.chromium.org/2777223004
> Cr-Commit-Position: refs/heads/master@{#462419}
> Committed: https://chromium.googlesource.com/chromium/src/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786

TBR=boliu@chromium.org,tedchoc@chromium.org,jinsukkim@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=662908,626765,620172

Review-Url: https://codereview.chromium.org/2803203003
Cr-Commit-Position: refs/heads/master@{#463053}
parent e3e4a4e4
...@@ -132,9 +132,7 @@ public class AwContentsGarbageCollectionTest extends AwTestBase { ...@@ -132,9 +132,7 @@ public class AwContentsGarbageCollectionTest extends AwTestBase {
resultReceivers[i] = runTestOnUiThreadAndGetResult(new Callable<ResultReceiver>() { resultReceivers[i] = runTestOnUiThreadAndGetResult(new Callable<ResultReceiver>() {
@Override @Override
public ResultReceiver call() throws Exception { public ResultReceiver call() throws Exception {
return containerView.getContentViewCore() return containerView.getContentViewCore().getNewShowKeyboardReceiver();
.getImeAdapterForTest()
.getNewShowKeyboardReceiver();
} }
}); });
} }
...@@ -275,3 +273,4 @@ public class AwContentsGarbageCollectionTest extends AwTestBase { ...@@ -275,3 +273,4 @@ public class AwContentsGarbageCollectionTest extends AwTestBase {
assertTrue(criteria.isSatisfied()); assertTrue(criteria.isSatisfied());
} }
} }
...@@ -94,7 +94,6 @@ import org.chromium.content.browser.ContentViewClient; ...@@ -94,7 +94,6 @@ import org.chromium.content.browser.ContentViewClient;
import org.chromium.content.browser.ContentViewCore; import org.chromium.content.browser.ContentViewCore;
import org.chromium.content.browser.crypto.CipherFactory; import org.chromium.content.browser.crypto.CipherFactory;
import org.chromium.content_public.browser.GestureStateListener; import org.chromium.content_public.browser.GestureStateListener;
import org.chromium.content_public.browser.ImeEventObserver;
import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.common.BrowserControlsState; import org.chromium.content_public.common.BrowserControlsState;
...@@ -374,6 +373,20 @@ public class Tab ...@@ -374,6 +373,20 @@ public class Tab
private boolean mIsAllowedToReturnToExternalApp; private boolean mIsAllowedToReturnToExternalApp;
private class TabContentViewClient extends ContentViewClient { private class TabContentViewClient extends ContentViewClient {
@Override
public void onImeEvent() {
// Some text was set in the page. Don't reuse it if a tab is
// open from the same external application, we might lose some
// user data.
mAppAssociatedWith = null;
}
@Override
public void onFocusedNodeEditabilityChanged(boolean editable) {
if (getFullscreenManager() == null) return;
updateFullscreenEnabledState();
}
@Override @Override
public int getSystemWindowInsetBottom() { public int getSystemWindowInsetBottom() {
ChromeActivity activity = getActivity(); ChromeActivity activity = getActivity();
...@@ -1314,22 +1327,6 @@ public class Tab ...@@ -1314,22 +1327,6 @@ public class Tab
setContentViewCore(contentViewCore); setContentViewCore(contentViewCore);
} }
mContentViewCore.addImeEventObserver(new ImeEventObserver() {
@Override
public void onImeEvent() {
// Some text was set in the page. Don't reuse it if a tab is
// open from the same external application, we might lose some
// user data.
mAppAssociatedWith = null;
}
@Override
public void onNodeAttributeUpdated(boolean editable, boolean password) {
if (getFullscreenManager() == null) return;
updateFullscreenEnabledState();
}
});
if (!creatingWebContents && webContents.isLoadingToDifferentDocument()) { if (!creatingWebContents && webContents.isLoadingToDifferentDocument()) {
didStartPageLoad(webContents.getUrl(), false); didStartPageLoad(webContents.getUrl(), false);
} }
......
...@@ -1087,6 +1087,28 @@ void ContentViewCoreImpl::WasResized(JNIEnv* env, ...@@ -1087,6 +1087,28 @@ void ContentViewCoreImpl::WasResized(JNIEnv* env,
SendScreenRectsAndResizeWidget(); SendScreenRectsAndResizeWidget();
} }
void ContentViewCoreImpl::UpdateImeAdapter(int text_input_type,
int text_input_flags,
int text_input_mode,
const std::string& text,
int selection_start,
int selection_end,
int composition_start,
int composition_end,
bool show_ime_if_needed,
bool reply_to_request) {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
if (obj.is_null())
return;
ScopedJavaLocalRef<jstring> jstring_text = ConvertUTF8ToJavaString(env, text);
Java_ContentViewCore_updateImeAdapter(
env, obj, text_input_type, text_input_flags, text_input_mode,
jstring_text, selection_start, selection_end, composition_start,
composition_end, show_ime_if_needed, reply_to_request);
}
void ContentViewCoreImpl::SetAccessibilityEnabled( void ContentViewCoreImpl::SetAccessibilityEnabled(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>& obj, const JavaParamRef<jobject>& obj,
......
...@@ -263,6 +263,17 @@ class ContentViewCoreImpl : public ContentViewCore, ...@@ -263,6 +263,17 @@ class ContentViewCoreImpl : public ContentViewCore,
bool is_mobile_optimized_hint, bool is_mobile_optimized_hint,
const gfx::SelectionBound& selection_start); const gfx::SelectionBound& selection_start);
void UpdateImeAdapter(int text_input_type,
int text_input_flags,
int text_input_mode,
const std::string& text,
int selection_start,
int selection_end,
int composition_start,
int composition_end,
bool show_ime_if_needed,
bool reply_to_request);
bool HasFocus(); bool HasFocus();
void RequestDisallowInterceptTouchEvent(); void RequestDisallowInterceptTouchEvent();
void OnGestureEventAck(const blink::WebGestureEvent& event, void OnGestureEventAck(const blink::WebGestureEvent& event,
......
...@@ -32,7 +32,6 @@ ...@@ -32,7 +32,6 @@
using base::android::AttachCurrentThread; using base::android::AttachCurrentThread;
using base::android::ConvertJavaStringToUTF16; using base::android::ConvertJavaStringToUTF16;
using base::android::ConvertUTF8ToJavaString;
using base::android::JavaParamRef; using base::android::JavaParamRef;
using base::android::ScopedJavaLocalRef; using base::android::ScopedJavaLocalRef;
...@@ -188,21 +187,6 @@ void ImeAdapterAndroid::UpdateRenderProcessConnection( ...@@ -188,21 +187,6 @@ void ImeAdapterAndroid::UpdateRenderProcessConnection(
} }
} }
void ImeAdapterAndroid::UpdateState(const TextInputState& state) {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ime_adapter_.get(env);
if (obj.is_null())
return;
ScopedJavaLocalRef<jstring> jstring_text =
ConvertUTF8ToJavaString(env, state.value);
Java_ImeAdapter_updateState(env, obj, static_cast<int>(state.type),
state.flags, state.mode, state.show_ime_if_needed,
jstring_text, state.selection_start,
state.selection_end, state.composition_start,
state.composition_end, state.reply_to_request);
}
bool ImeAdapterAndroid::SendKeyEvent( bool ImeAdapterAndroid::SendKeyEvent(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>&, const JavaParamRef<jobject>&,
......
...@@ -26,7 +26,6 @@ namespace content { ...@@ -26,7 +26,6 @@ namespace content {
class RenderFrameHost; class RenderFrameHost;
class RenderWidgetHostImpl; class RenderWidgetHostImpl;
class RenderWidgetHostViewAndroid; class RenderWidgetHostViewAndroid;
struct TextInputState;
// This class is in charge of dispatching key events from the java side // This class is in charge of dispatching key events from the java side
// and forward to renderer along with input method results via // and forward to renderer along with input method results via
...@@ -106,8 +105,6 @@ class CONTENT_EXPORT ImeAdapterAndroid : public WebContentsObserver { ...@@ -106,8 +105,6 @@ class CONTENT_EXPORT ImeAdapterAndroid : public WebContentsObserver {
void DidDetachInterstitialPage() override; void DidDetachInterstitialPage() override;
void WebContentsDestroyed() override; void WebContentsDestroyed() override;
void UpdateState(const TextInputState& state);
private: private:
RenderWidgetHostImpl* GetFocusedWidget(); RenderWidgetHostImpl* GetFocusedWidget();
RenderFrameHost* GetFocusedFrame(); RenderFrameHost* GetFocusedFrame();
......
...@@ -719,10 +719,13 @@ void RenderWidgetHostViewAndroid::OnUpdateTextInputStateCalled( ...@@ -719,10 +719,13 @@ void RenderWidgetHostViewAndroid::OnUpdateTextInputStateCalled(
? *GetTextInputManager()->GetTextInputState() ? *GetTextInputManager()->GetTextInputState()
: TextInputState(); : TextInputState();
if (!ime_adapter_android_ || is_in_vr_) if (!content_view_core_ || is_in_vr_)
return; return;
ime_adapter_android_->UpdateState(state); content_view_core_->UpdateImeAdapter(
static_cast<int>(state.type), state.flags, state.mode, state.value,
state.selection_start, state.selection_end, state.composition_start,
state.composition_end, state.show_ime_if_needed, state.reply_to_request);
} }
void RenderWidgetHostViewAndroid::OnImeCompositionRangeChanged( void RenderWidgetHostViewAndroid::OnImeCompositionRangeChanged(
......
...@@ -153,7 +153,6 @@ android_library("content_java") { ...@@ -153,7 +153,6 @@ android_library("content_java") {
"java/src/org/chromium/content/browser/SpeechRecognition.java", "java/src/org/chromium/content/browser/SpeechRecognition.java",
"java/src/org/chromium/content/browser/TracingControllerAndroid.java", "java/src/org/chromium/content/browser/TracingControllerAndroid.java",
"java/src/org/chromium/content/browser/ViewPositionObserver.java", "java/src/org/chromium/content/browser/ViewPositionObserver.java",
"java/src/org/chromium/content/browser/ViewUtils.java",
"java/src/org/chromium/content/browser/WindowAndroidProvider.java", "java/src/org/chromium/content/browser/WindowAndroidProvider.java",
"java/src/org/chromium/content/browser/WindowAndroidChangedObserver.java", "java/src/org/chromium/content/browser/WindowAndroidChangedObserver.java",
"java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityManager.java", "java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityManager.java",
...@@ -224,7 +223,6 @@ android_library("content_java") { ...@@ -224,7 +223,6 @@ android_library("content_java") {
"java/src/org/chromium/content_public/browser/ContentBitmapCallback.java", "java/src/org/chromium/content_public/browser/ContentBitmapCallback.java",
"java/src/org/chromium/content_public/browser/GestureStateListener.java", "java/src/org/chromium/content_public/browser/GestureStateListener.java",
"java/src/org/chromium/content_public/browser/ImageDownloadCallback.java", "java/src/org/chromium/content_public/browser/ImageDownloadCallback.java",
"java/src/org/chromium/content_public/browser/ImeEventObserver.java",
"java/src/org/chromium/content_public/browser/InterfaceRegistrar.java", "java/src/org/chromium/content_public/browser/InterfaceRegistrar.java",
"java/src/org/chromium/content_public/browser/JavaScriptCallback.java", "java/src/org/chromium/content_public/browser/JavaScriptCallback.java",
"java/src/org/chromium/content_public/browser/LoadUrlParams.java", "java/src/org/chromium/content_public/browser/LoadUrlParams.java",
...@@ -447,6 +445,7 @@ android_library("content_javatests") { ...@@ -447,6 +445,7 @@ android_library("content_javatests") {
"javatests/src/org/chromium/content/browser/input/ImeTestUtils.java", "javatests/src/org/chromium/content/browser/input/ImeTestUtils.java",
"javatests/src/org/chromium/content/browser/input/InputDialogContainerTest.java", "javatests/src/org/chromium/content/browser/input/InputDialogContainerTest.java",
"javatests/src/org/chromium/content/browser/input/SelectPopupTest.java", "javatests/src/org/chromium/content/browser/input/SelectPopupTest.java",
"javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java",
"javatests/src/org/chromium/content/browser/picker/DateTimePickerDialogTest.java", "javatests/src/org/chromium/content/browser/picker/DateTimePickerDialogTest.java",
"javatests/src/org/chromium/content/browser/webcontents/AccessibilitySnapshotTest.java", "javatests/src/org/chromium/content/browser/webcontents/AccessibilitySnapshotTest.java",
"javatests/src/org/chromium/content/browser/webcontents/WebContentsTest.java", "javatests/src/org/chromium/content/browser/webcontents/WebContentsTest.java",
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.content.browser;
import android.view.View;
/**
* A utility class that has helper methods for Android view.
*/
public final class ViewUtils {
// Prevent instantiation.
private ViewUtils() {}
/**
* @return {@code true} if the given view has a focus.
*/
public static boolean hasFocus(View view) {
// If the container view is not focusable, we consider it always focused from
// Chromium's point of view.
return !view.isFocusable() ? true : view.hasFocus();
}
}
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.content_public.browser;
/**
* Interface for the classes that need to be notified of IME changes.
*/
public interface ImeEventObserver {
/**
* Called to notify the delegate about synthetic/real key events before sending to renderer.
*/
void onImeEvent();
/**
* Called when the focused node attribute is updated.
* @param editable {@code true} if the node becomes editable; else {@code false}.
* @param password indicates the node is of type password if {@code true}.
*/
void onNodeAttributeUpdated(boolean editable, boolean password);
}
...@@ -16,6 +16,7 @@ import android.view.View; ...@@ -16,6 +16,7 @@ import android.view.View;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.content.browser.input.ImeAdapter; import org.chromium.content.browser.input.ImeAdapter;
import org.chromium.content.browser.input.TestImeAdapterDelegate;
import org.chromium.content.browser.test.util.TestInputMethodManagerWrapper; import org.chromium.content.browser.test.util.TestInputMethodManagerWrapper;
import org.chromium.content_shell_apk.ContentShellTestBase; import org.chromium.content_shell_apk.ContentShellTestBase;
...@@ -78,21 +79,25 @@ public class PopupZoomerTest extends ContentShellTestBase { ...@@ -78,21 +79,25 @@ public class PopupZoomerTest extends ContentShellTestBase {
@Override @Override
public void setUp() throws Exception { public void setUp() throws Exception {
super.setUp(); super.setUp();
mPopupZoomer = createPopupZoomerForTest(getInstrumentation().getTargetContext());
mContentViewCore = new ContentViewCore(getActivity(), "");
final Context context = getActivity(); final Context context = getActivity();
ThreadUtils.runOnUiThreadBlocking(new Runnable() { ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override @Override
public void run() { public void run() {
mPopupZoomer = createPopupZoomerForTest(getInstrumentation().getTargetContext());
mContentViewCore = new ContentViewCore(context, ""); mContentViewCore = new ContentViewCore(context, "");
ImeAdapter imeAdapter = new ImeAdapter(
getContentViewCore().getContainerView().getResources().getConfiguration(),
getContentViewCore().getWebContents(),
new TestInputMethodManagerWrapper(mContentViewCore),
new TestImeAdapterDelegate(getContentViewCore().getContainerView()));
mContentViewCore.setSelectionPopupControllerForTesting(new SelectionPopupController( mContentViewCore.setSelectionPopupControllerForTesting(new SelectionPopupController(
context, null, null, null, mContentViewCore.getRenderCoordinates())); context, null, null, null, mContentViewCore.getRenderCoordinates()));
mContentViewCore.setImeAdapterForTest(
new ImeAdapter(getContentViewCore().getWebContents(),
getContentViewCore().getContainerView(),
new TestInputMethodManagerWrapper(mContentViewCore)));
mPopupZoomer = createPopupZoomerForTest(getInstrumentation().getTargetContext());
mContentViewCore.setPopupZoomerForTest(mPopupZoomer); mContentViewCore.setPopupZoomerForTest(mPopupZoomer);
mContentViewCore.setImeAdapterForTest(imeAdapter);
} }
}); });
} }
......
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.content.browser.input;
import android.os.ResultReceiver;
import android.view.View;
import org.chromium.content.browser.input.ImeAdapter.ImeAdapterDelegate;
/**
* An empty ImeAdapterDelegate used for testing.
*/
public class TestImeAdapterDelegate implements ImeAdapterDelegate {
private final View mView;
public TestImeAdapterDelegate(View view) {
mView = view;
}
@Override
public void onImeEvent() {}
@Override
public void onKeyboardBoundsUnchanged() {}
@Override
public boolean performContextMenuAction(int id) {
return false;
}
@Override
public View getAttachedView() {
return mView;
}
@Override
public ResultReceiver getNewShowKeyboardReceiver() {
return null;
}
}
\ No newline at end of file
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