Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(88)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2613863006: Add a ThreadLocal flag to replace the reflection in shouldTriggerDelayedOnCreateInputConnection() (Closed)

Created:
3 years, 11 months ago by yabinh
Modified:
3 years, 8 months ago
Reviewers:
Changwan Ryu
CC:
chromium-reviews, jam, darin-cc_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a ThreadLocal flag to replace the reflection in shouldTriggerDelayedOnCreateInputConnection() shouldTriggerDelayedOnCreateInputConnection() uses reflection to distinguish the callers. It's pretty nasty. This CL passes a ThreadLocal flag from the callers to it to achieve the same purpose. BUG=636474

Patch Set 1 #

Patch Set 2 : Remove @UsedByReflection #

Patch Set 3 : SuppressFBWarnings(CHROMIUM_SYNCHRONIZED_METHOD) #

Patch Set 4 : assert #

Patch Set 5 : Fix 2 mockito tests #

Patch Set 6 : Sync #

Patch Set 7 : fix cast error #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -4 lines) Patch
M content/public/android/java/src/org/chromium/content/browser/ContentView.java View 1 2 3 chunks +16 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 2 3 chunks +16 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java View 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java View 1 2 4 chunks +17 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java View 1 2 3 5 chunks +22 lines, -0 lines 1 comment Download
M content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java View 1 2 3 4 5 6 3 chunks +4 lines, -2 lines 2 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java View 1 2 3 4 5 2 chunks +17 lines, -0 lines 0 comments Download
M content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java View 1 2 3 4 3 chunks +5 lines, -0 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java View 1 4 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 27 (25 generated)
yabinh
https://codereview.chromium.org/2613863006/diff/140001/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java File content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java (right): https://codereview.chromium.org/2613863006/diff/140001/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java#newcode145 content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java:145: Keep the old method for debugging. https://codereview.chromium.org/2613863006/diff/140001/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java File content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java ...
3 years, 11 months ago (2017-01-09 07:33:11 UTC) #25
Changwan Ryu
3 years, 11 months ago (2017-01-10 06:28:25 UTC) #27 
https://codereview.chromium.org/2613863006/diff/140001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java
(right):

https://codereview.chromium.org/2613863006/diff/140001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java:97:
}
On 2017/01/09 07:33:11, yabinh wrote:
> We can't import 'org.chromium.android_webview.test.AwTestContainerView' into
> ThreadedInputConnectionProxyView.java, so we can't do the similar trick here:
> 
> '''
>                 if (mContainerView instanceof AwTestContainerView) {
>                     ((AwTestContainerView) mContainerView)
>                             .setTriggerDelayedOnCreateInputConnection(false);
>                 }
> '''
> 
> 
> Here is part of the call stack for WebKitHitTestTest.testEditTextType:
> 
> '''
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.content.browser.input.ThreadedInputConnectionFactory.initializeAndGet(ThreadedInputConnectionFactory.java:147)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.content.browser.input.ThreadedInputConnectionFactory.initializeAndGet(ThreadedInputConnectionFactory.java:22)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.content.browser.input.ImeAdapter.onCreateInputConnection(ImeAdapter.java:199)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.content.browser.ContentViewCore.onCreateInputConnection(ContentViewCore.java:1398)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.android_webview.AwContents$AwViewMethodsImpl.onCreateInputConnection(AwContents.java:3056)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.android_webview.AwContents.onCreateInputConnection(AwContents.java:1929)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.android_webview.test.AwTestContainerView.onCreateInputConnection(AwTestContainerView.java:355)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.content.browser.input.ThreadedInputConnectionProxyView$1.call(ThreadedInputConnectionProxyView.java:98)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
>
org.chromium.content.browser.input.ThreadedInputConnectionProxyView$1.call(ThreadedInputConnectionProxyView.java:92)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
> java.util.concurrent.FutureTask.run(FutureTask.java:237)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
> android.os.Handler.handleCallback(Handler.java:739)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
> android.os.Handler.dispatchMessage(Handler.java:95)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
> android.os.Looper.loop(Looper.java:148)
> 01-09 15:17:41.580 16855 16855 E cr_hyb  :
> android.app.ActivityThread.main(ActivityThread.java:5417)
> 01-09 15:17:41.581 16855 16855 E cr_hyb  :
> java.lang.reflect.Method.invoke(Native Method)
> '''

Since ProxyView is owned by Factory, could you just pass Factory as a parameter
to a constructor instead? Then you don't need a lot of plumbing. Then you can do
the following:

mFactory.setCalledByProxyView(true);
InputConnection connection = mContainerView.onCreateInputConnection(outAttrs);
mFactory.setCalledByProxyView(false);
return connection;

Powered by Google App Engine
This is Rietveld 408576698