Skip to content

GitLab

T
tangled
Commit fbc7c1e4 authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot
Browse files
Options

TextSuggestionHost on WebContentsUserData 

This CL moves Java TextSuggestionHost to be managed by
WebContentsUserData so ContentViewCore doesn't have to keep
a reference to the object, and the object can be obtained
easily by TextSuggestionHost.fromWebContents(). It helps
decouple the class from CVC.

Bug: 789000
Change-Id: Iefdeb0a727fc975323b7f8f24d4eb187ad38601b
Reviewed-on: https://chromium-review.googlesource.com/892180Reviewed-by: default avatarRyan Landay <rlanday@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533068}
parent b17acb28
Hide whitespace changes
Inline Side-by-side
Showing with 121 additions and 103 deletions
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
View file @ fbc7c1e4
......@@ -13,7 +13,6 @@ import android.view.ViewGroup;
import org.chromium.base.VisibleForTesting;
import org.chromium.content.browser.input.SelectPopup;
import org.chromium.content.browser.input.TextSuggestionHost;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.ViewAndroidDelegate;
import org.chromium.ui.base.WindowAndroid;
......@@ -378,15 +377,6 @@ public interface ContentViewCore {
// Test-only methods
/**
* @return The TextSuggestionHost that handles displaying the text suggestion menu.
*/
@VisibleForTesting
TextSuggestionHost getTextSuggestionHostForTesting();
@VisibleForTesting
void setTextSuggestionHostForTesting(TextSuggestionHost textSuggestionHost);
/**
* @return The amount of the top controls height if controls are in the state
* of shrinking Blink's view size, otherwise 0.
......
content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
View file @ fbc7c1e4
......@@ -158,8 +158,6 @@ public class ContentViewCoreImpl
private SelectPopup mSelectPopup;
private long mNativeSelectPopupSourceFrame;
private TextSuggestionHost mTextSuggestionHost;
// Cached copy of all positions and scales as reported by the renderer.
private RenderCoordinates mRenderCoordinates;
......@@ -253,23 +251,11 @@ public class ContentViewCoreImpl
return nativeGetJavaWindowAndroid(mNativeContentViewCore);
}
@VisibleForTesting
@Override
public TextSuggestionHost getTextSuggestionHostForTesting() {
return mTextSuggestionHost;
}
@VisibleForTesting
void setWebContentsForTesting(WebContentsImpl webContents) {
mWebContents = webContents;
}
@VisibleForTesting
@Override
public void setTextSuggestionHostForTesting(TextSuggestionHost textSuggestionHost) {
mTextSuggestionHost = textSuggestionHost;
}
/**
* Add {@link WindowAndroidChangeObserver} object.
* @param observer Observer instance to add.
......@@ -321,9 +307,9 @@ public class ContentViewCoreImpl
mWebContents, mContainerView, new InputMethodManagerWrapper(mContext));
imeAdapter.addEventObserver(this);
imeAdapter.addEventObserver(TapDisambiguator.create(mContext, mWebContents, containerView));
mTextSuggestionHost =
new TextSuggestionHost(mContext, mWebContents, windowAndroid, mContainerView);
addWindowAndroidChangedObserver(mTextSuggestionHost);
TextSuggestionHost textSuggestionHost =
TextSuggestionHost.create(mContext, mWebContents, windowAndroid, containerView);
addWindowAndroidChangedObserver(textSuggestionHost);
mWebContentsObserver = new ContentViewWebContentsObserver(this);
......@@ -334,7 +320,7 @@ public class ContentViewCoreImpl
mWindowEventObservers.addObserver(controller);
mWindowEventObservers.addObserver(getGestureListenerManager());
mWindowEventObservers.addObserver(mTextSuggestionHost);
mWindowEventObservers.addObserver(textSuggestionHost);
mWindowEventObservers.addObserver(imeAdapter);
mWindowEventObservers.addObserver(wcax);
}
......@@ -385,7 +371,7 @@ public class ContentViewCoreImpl
if (mContainerView != null) {
hideSelectPopupWithCancelMessage();
getImeAdapter().setContainerView(containerView);
mTextSuggestionHost.setContainerView(containerView);
getTextSuggestionHost().setContainerView(containerView);
}
mContainerView = containerView;
......@@ -416,6 +402,10 @@ public class ContentViewCoreImpl
return WebContentsAccessibilityImpl.fromWebContents(mWebContents);
}
private TextSuggestionHost getTextSuggestionHost() {
return TextSuggestionHost.fromWebContents(mWebContents);
}
@CalledByNative
private void onNativeContentViewCoreDestroyed(long nativeContentViewCore) {
assert nativeContentViewCore == mNativeContentViewCore;
......@@ -436,7 +426,7 @@ public class ContentViewCoreImpl
mWebContentsObserver.destroy();
mWebContentsObserver = null;
getImeAdapter().resetAndHideKeyboard();
removeWindowAndroidChangedObserver(mTextSuggestionHost);
removeWindowAndroidChangedObserver(getTextSuggestionHost());
mWindowEventObservers.clear();
hidePopupsAndPreserveSelection();
mWebContents = null;
......@@ -558,8 +548,6 @@ public class ContentViewCoreImpl
private void hidePopupsAndClearSelection() {
if (mWebContents != null) {
getSelectionPopupController().destroyActionModeAndUnselect();
destroyPastePopup();
getTapDisambiguator().hidePopup(false);
mWebContents.dismissTextHandles();
}
hidePopups();
......@@ -569,15 +557,17 @@ public class ContentViewCoreImpl
private void hidePopupsAndPreserveSelection() {
if (mWebContents != null) {
getSelectionPopupController().destroyActionModeAndKeepSelection();
destroyPastePopup();
getTapDisambiguator().hidePopup(false);
}
hidePopups();
}
private void hidePopups() {
if (mWebContents != null) {
destroyPastePopup();
getTapDisambiguator().hidePopup(false);
getTextSuggestionHost().hidePopups();
}
hideSelectPopupWithCancelMessage();
mTextSuggestionHost.hidePopups();
}
private void restoreSelectionPopupsIfNecessary() {
......@@ -1141,7 +1131,7 @@ public class ContentViewCoreImpl
hidePopupsAndPreserveSelection();
getSelectionPopupController().showActionModeOrClearOnFailure();
}
mTextSuggestionHost.hidePopups();
getTextSuggestionHost().hidePopups();
int rotationDegrees = 0;
switch (rotation) {
......
content/public/android/java/src/org/chromium/content/browser/input/TextSuggestionHost.java
View file @ fbc7c1e4
......@@ -13,6 +13,8 @@ import org.chromium.base.annotations.JNINamespace;
import org.chromium.content.browser.WindowAndroidChangedObserver;
import org.chromium.content.browser.WindowEventObserver;
import org.chromium.content.browser.webcontents.WebContentsImpl;
import org.chromium.content.browser.webcontents.WebContentsUserData;
import org.chromium.content.browser.webcontents.WebContentsUserData.UserDataFactory;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid;
......@@ -24,9 +26,9 @@ import org.chromium.ui.base.WindowAndroid;
@JNINamespace("content")
public class TextSuggestionHost implements WindowEventObserver, WindowAndroidChangedObserver {
private long mNativeTextSuggestionHost;
private final Context mContext;
private final WebContentsImpl mWebContents;
private Context mContext;
private View mContainerView;
private boolean mIsAttachedToWindow;
private WindowAndroid mWindowAndroid;
......@@ -34,13 +36,58 @@ public class TextSuggestionHost implements WindowEventObserver, WindowAndroidCha
private SpellCheckPopupWindow mSpellCheckPopupWindow;
private TextSuggestionsPopupWindow mTextSuggestionsPopupWindow;
public TextSuggestionHost(
Context context, WebContentsImpl webContents, WindowAndroid windowAndroid, View view) {
private boolean mInitialized;
private static final class UserDataFactoryLazyHolder {
private static final UserDataFactory<TextSuggestionHost> INSTANCE = TextSuggestionHost::new;
}
/**
* Create {@link TextSuggestionHost} instance.
* @param context Context for action mode.
* @param webContents WebContents instance.
* @param windowAndroid The current WindowAndroid instance.
* @param view Container view.
*/
public static TextSuggestionHost create(
Context context, WebContents webContents, WindowAndroid windowAndroid, View view) {
TextSuggestionHost host = WebContentsUserData.fromWebContents(
webContents, TextSuggestionHost.class, UserDataFactoryLazyHolder.INSTANCE);
assert host != null;
assert !host.initialized();
host.init(context, windowAndroid, view);
return host;
}
/**
* Get {@link TextSuggestionHost} object used for the give WebContents.
* {@link #create()} should precede any calls to this.
* @param webContents {@link WebContents} object.
* @return {@link TextSuggestionHost} object. {@code null} if not available because
* {@link #create()} is not called yet.
*/
public static TextSuggestionHost fromWebContents(WebContents webContents) {
return WebContentsUserData.fromWebContents(webContents, TextSuggestionHost.class, null);
}
/**
* Create {@link TextSuggestionHost} instance.
* @param webContents WebContents instance.
*/
public TextSuggestionHost(WebContents webContents) {
mWebContents = (WebContentsImpl) webContents;
}
private void init(Context context, WindowAndroid windowAndroid, View view) {
mContext = context;
mWebContents = webContents;
mWindowAndroid = windowAndroid;
mContainerView = view;
mNativeTextSuggestionHost = nativeInit(webContents);
mNativeTextSuggestionHost = nativeInit(mWebContents);
mInitialized = true;
}
private boolean initialized() {
return mInitialized;
}
private float getContentOffsetYPix() {
......
content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
View file @ fbc7c1e4
......@@ -23,7 +23,6 @@ import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.Feature;
import org.chromium.content.browser.input.TextSuggestionHost;
import org.chromium.content.browser.test.ContentJUnit4ClassRunner;
import org.chromium.content.browser.test.util.TestInputMethodManagerWrapper;
import org.chromium.content.browser.webcontents.WebContentsImpl;
......@@ -108,8 +107,6 @@ public class PopupZoomerTest {
mPopupZoomer = createPopupZoomerForTest(
InstrumentationRegistry.getTargetContext(), containerView);
TapDisambiguator.fromWebContents(webContents).setPopupZoomerForTest(mPopupZoomer);
mContentViewCore.setTextSuggestionHostForTesting(new TextSuggestionHost(
context, (WebContentsImpl) webContents, null, containerView));
}
});
}
......
content/public/android/javatests/src/org/chromium/content/browser/input/TextSuggestionMenuTest.java
View file @ fbc7c1e4
......@@ -55,7 +55,7 @@ public class TextSuggestionMenuTest {
public void testDeleteWordMarkedWithSuggestionMarker()
throws InterruptedException, Throwable, TimeoutException {
final ContentViewCore cvc = mRule.getContentViewCore();
WebContents webContents = cvc.getWebContents();
WebContents webContents = mRule.getWebContents();
DOMUtils.focusNode(webContents, "div");
......@@ -66,22 +66,22 @@ public class TextSuggestionMenuTest {
mRule.commitText(textToCommit, 1);
DOMUtils.clickNode(cvc, "div");
waitForMenuToShow(cvc);
waitForMenuToShow(webContents);
TouchCommon.singleClickView(getDeleteButton(cvc));
TouchCommon.singleClickView(getDeleteButton(webContents));
CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override
public boolean isSatisfied() {
try {
return DOMUtils.getNodeContents(cvc.getWebContents(), "div").equals("");
return DOMUtils.getNodeContents(webContents, "div").equals("");
} catch (InterruptedException | TimeoutException e) {
return false;
}
}
});
waitForMenuToHide(cvc);
waitForMenuToHide(webContents);
}
@Test
......@@ -89,7 +89,7 @@ public class TextSuggestionMenuTest {
public void testDeleteWordMarkedWithSpellingMarker()
throws InterruptedException, Throwable, TimeoutException {
final ContentViewCore cvc = mRule.getContentViewCore();
WebContents webContents = cvc.getWebContents();
WebContents webContents = mRule.getWebContents();
DOMUtils.focusNode(webContents, "div");
......@@ -121,29 +121,29 @@ public class TextSuggestionMenuTest {
+ "internals.setMarker(document, range, 'spelling');");
DOMUtils.clickNode(cvc, "div");
waitForMenuToShow(cvc);
waitForMenuToShow(webContents);
TouchCommon.singleClickView(getDeleteButton(cvc));
TouchCommon.singleClickView(getDeleteButton(webContents));
CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override
public boolean isSatisfied() {
try {
return DOMUtils.getNodeContents(cvc.getWebContents(), "div").equals("");
return DOMUtils.getNodeContents(mRule.getWebContents(), "div").equals("");
} catch (InterruptedException | TimeoutException e) {
return false;
}
}
});
waitForMenuToHide(cvc);
waitForMenuToHide(webContents);
}
@Test
@LargeTest
public void testApplySuggestion() throws InterruptedException, Throwable, TimeoutException {
final ContentViewCore cvc = mRule.getContentViewCore();
WebContents webContents = cvc.getWebContents();
WebContents webContents = mRule.getWebContents();
DOMUtils.focusNode(webContents, "div");
......@@ -187,27 +187,27 @@ public class TextSuggestionMenuTest {
});
DOMUtils.clickNode(cvc, "span");
waitForMenuToShow(cvc);
waitForMenuToShow(webContents);
// There should be 5 child views: 4 suggestions plus the list footer.
Assert.assertEquals(5, getSuggestionList(cvc).getChildCount());
Assert.assertEquals(5, getSuggestionList(webContents).getChildCount());
Assert.assertEquals(
"hello suggestion1", ((TextView) getSuggestionButton(cvc, 0)).getText().toString());
Assert.assertEquals(
"hello suggestion2", ((TextView) getSuggestionButton(cvc, 1)).getText().toString());
Assert.assertEquals(
"suggestion3", ((TextView) getSuggestionButton(cvc, 2)).getText().toString());
Assert.assertEquals(
"suggestion4", ((TextView) getSuggestionButton(cvc, 3)).getText().toString());
Assert.assertEquals("hello suggestion1",
((TextView) getSuggestionButton(webContents, 0)).getText().toString());
Assert.assertEquals("hello suggestion2",
((TextView) getSuggestionButton(webContents, 1)).getText().toString());
Assert.assertEquals("suggestion3",
((TextView) getSuggestionButton(webContents, 2)).getText().toString());
Assert.assertEquals("suggestion4",
((TextView) getSuggestionButton(webContents, 3)).getText().toString());
TouchCommon.singleClickView(getSuggestionButton(cvc, 2));
TouchCommon.singleClickView(getSuggestionButton(webContents, 2));
CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override
public boolean isSatisfied() {
try {
return DOMUtils.getNodeContents(cvc.getWebContents(), "div")
return DOMUtils.getNodeContents(mRule.getWebContents(), "div")
.equals("suggestion3");
} catch (InterruptedException | TimeoutException e) {
return false;
......@@ -215,7 +215,7 @@ public class TextSuggestionMenuTest {
}
});
waitForMenuToHide(cvc);
waitForMenuToHide(webContents);
}
@Test
......@@ -223,7 +223,7 @@ public class TextSuggestionMenuTest {
public void testApplyMisspellingSuggestion()
throws InterruptedException, Throwable, TimeoutException {
final ContentViewCore cvc = mRule.getContentViewCore();
WebContents webContents = cvc.getWebContents();
WebContents webContents = mRule.getWebContents();
DOMUtils.focusNode(webContents, "div");
......@@ -237,21 +237,21 @@ public class TextSuggestionMenuTest {
mRule.commitText(textToCommit, 1);
DOMUtils.clickNode(cvc, "span");
waitForMenuToShow(cvc);
waitForMenuToShow(webContents);
// There should be 2 child views: 1 suggestion plus the list footer.
Assert.assertEquals(2, getSuggestionList(cvc).getChildCount());
Assert.assertEquals(2, getSuggestionList(webContents).getChildCount());
Assert.assertEquals(
"replacement", ((TextView) getSuggestionButton(cvc, 0)).getText().toString());
Assert.assertEquals("replacement",
((TextView) getSuggestionButton(webContents, 0)).getText().toString());
TouchCommon.singleClickView(getSuggestionButton(cvc, 0));
TouchCommon.singleClickView(getSuggestionButton(webContents, 0));
CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override
public boolean isSatisfied() {
try {
return DOMUtils.getNodeContents(cvc.getWebContents(), "div")
return DOMUtils.getNodeContents(mRule.getWebContents(), "div")
.equals("replacement");
} catch (InterruptedException | TimeoutException e) {
return false;
......@@ -259,7 +259,7 @@ public class TextSuggestionMenuTest {
}
});
waitForMenuToHide(cvc);
waitForMenuToHide(webContents);
// Verify that the suggestion marker was replaced.
Assert.assertEquals("0",
......@@ -272,7 +272,7 @@ public class TextSuggestionMenuTest {
@LargeTest
public void suggestionMenuDismissal() throws InterruptedException, Throwable, TimeoutException {
final ContentViewCore cvc = mRule.getContentViewCore();
WebContents webContents = cvc.getWebContents();
WebContents webContents = mRule.getWebContents();
DOMUtils.focusNode(webContents, "div");
......@@ -283,24 +283,24 @@ public class TextSuggestionMenuTest {
mRule.commitText(textToCommit, 1);
DOMUtils.clickNode(cvc, "div");
waitForMenuToShow(cvc);
waitForMenuToShow(webContents);
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
cvc.getTextSuggestionHostForTesting()
TextSuggestionHost.fromWebContents(webContents)
.getTextSuggestionsPopupWindowForTesting()
.dismiss();
}
});
waitForMenuToHide(cvc);
waitForMenuToHide(webContents);
}
private void waitForMenuToShow(ContentViewCore cvc) {
private void waitForMenuToShow(WebContents webContents) {
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
View deleteButton = getDeleteButton(cvc);
View deleteButton = getDeleteButton(webContents);
if (deleteButton == null) {
return false;
}
......@@ -314,32 +314,35 @@ public class TextSuggestionMenuTest {
});
}
private void waitForMenuToHide(ContentViewCore cvc) {
private void waitForMenuToHide(WebContents webContents) {
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
SuggestionsPopupWindow suggestionsPopupWindow =
cvc.getTextSuggestionHostForTesting()
TextSuggestionHost.fromWebContents(webContents)
.getTextSuggestionsPopupWindowForTesting();
SuggestionsPopupWindow spellCheckPopupWindow =
cvc.getTextSuggestionHostForTesting().getSpellCheckPopupWindowForTesting();
TextSuggestionHost.fromWebContents(webContents)
.getSpellCheckPopupWindowForTesting();
return suggestionsPopupWindow == null && spellCheckPopupWindow == null;
}
});
}
private View getContentView(ContentViewCore cvc) {
private View getContentView(WebContents webContents) {
SuggestionsPopupWindow suggestionsPopupWindow =
cvc.getTextSuggestionHostForTesting().getTextSuggestionsPopupWindowForTesting();
TextSuggestionHost.fromWebContents(webContents)
.getTextSuggestionsPopupWindowForTesting();
if (suggestionsPopupWindow != null) {
return suggestionsPopupWindow.getContentViewForTesting();
}
SuggestionsPopupWindow spellCheckPopupWindow =
cvc.getTextSuggestionHostForTesting().getSpellCheckPopupWindowForTesting();
TextSuggestionHost.fromWebContents(webContents)
.getSpellCheckPopupWindowForTesting();
if (spellCheckPopupWindow != null) {
return spellCheckPopupWindow.getContentViewForTesting();
......@@ -348,17 +351,17 @@ public class TextSuggestionMenuTest {
return null;
}
private ListView getSuggestionList(ContentViewCore cvc) {
View contentView = getContentView(cvc);
private ListView getSuggestionList(WebContents webContents) {
View contentView = getContentView(webContents);
return (ListView) contentView.findViewById(R.id.suggestionContainer);
}
private View getSuggestionButton(ContentViewCore cvc, int suggestionIndex) {
return getSuggestionList(cvc).getChildAt(suggestionIndex);
private View getSuggestionButton(WebContents webContents, int suggestionIndex) {
return getSuggestionList(webContents).getChildAt(suggestionIndex);
}
private View getDeleteButton(ContentViewCore cvc) {
View contentView = getContentView(cvc);
private View getDeleteButton(WebContents webContents) {
View contentView = getContentView(webContents);
if (contentView == null) {
return null;
}
......
content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewCore.java
View file @ fbc7c1e4
......@@ -13,7 +13,6 @@ import android.view.ViewGroup;
import org.chromium.content.browser.ContentViewCore;
import org.chromium.content.browser.ContentViewCore.InternalAccessDelegate;
import org.chromium.content.browser.input.SelectPopup;
import org.chromium.content.browser.input.TextSuggestionHost;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.ViewAndroidDelegate;
import org.chromium.ui.base.WindowAndroid;
......@@ -45,14 +44,6 @@ public class TestContentViewCore implements ContentViewCore {
return null;
}
@Override
public TextSuggestionHost getTextSuggestionHostForTesting() {
return null;
}
@Override
public void setTextSuggestionHostForTesting(TextSuggestionHost textSuggestionHost) {}
@Override
public void initialize(ViewAndroidDelegate viewDelegate,
InternalAccessDelegate internalDispatcher, WebContents webContents,
......
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