shouldTriggerDelayedOnCreateInputConnection walks through stack trace and checks class names with reflection |
|||||||
Issue descriptionshouldTriggerDelayedOnCreateInputConnection() in ThreadedInputConnectionFactory.java iterates through Thread.currentThread().getStackTrace() and checks for certain class names in the stack trace. This is a pretty nasty way to check our caller, especially when we start obfuscating our code (and we want to rename these classes). Ideally, we'd find another way to make this work. Perhaps we can have the callers of shouldTriggerDelayedOnCreateInputConnection set a thread-local static flag, and then have should...Connection check this flag. This code was introduced here: https://codereview.chromium.org/1278593004
,
Aug 11 2016
I don't have a cycle right now, when do we start obfucscating the code?
,
Aug 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47d9b4b24d614fb601fd695f30d911e3879dfaaa commit 47d9b4b24d614fb601fd695f30d911e3879dfaaa Author: smaier <smaier@chromium.org> Date: Thu Aug 11 14:15:50 2016 Adding @UsedByReflection annotation to 2 classes used by reflection BUG= 636474 Review-Url: https://codereview.chromium.org/2235923002 Cr-Commit-Position: refs/heads/master@{#411320} [modify] https://crrev.com/47d9b4b24d614fb601fd695f30d911e3879dfaaa/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/47d9b4b24d614fb601fd695f30d911e3879dfaaa/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java [modify] https://crrev.com/47d9b4b24d614fb601fd695f30d911e3879dfaaa/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
,
Jan 2 2017
yabinh@, do you have a cycle for this?
,
Jan 9 2017
I don't think it's a good idea to set a thread-local static flag. I have an experimental CL working on this, which even makes the code nastier. See https://codereview.chromium.org/2613863006. (The CL is not complete it yet. Some WebKitHitTestTest tests fails after applying the patch.) I can't figure out a better way. Maybe we'd better keep it as it is?
,
Apr 10 2017
yabinh@ has left the team. changwan@'s proposal on the code review: "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;"
,
Mar 21 2018
,
Mar 21 2018
My oversight about not fixing this in time... ctzsm@, could you take a look?
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/347a2a146b73e8e3f5272705f979917008b324c6 commit 347a2a146b73e8e3f5272705f979917008b324c6 Author: Shimi Zhang <ctzsm@chromium.org> Date: Wed May 02 23:03:21 2018 [IME] Remove a reflection in |ThreadedInputConnectionFactory| Previously we have a reflection check for not delaying onCreateInputConnection() for |ThreadedInputConnectionProxyView| and |TestInputMethodeManagerWrapper|. we replaced this reflection by setting a flag from caller directly and calls onCreateInputConnection(), and then reset the flag after. Bug: 636474 Change-Id: I7563f38d333a32ea9ccddec69446d1758275300f Reviewed-on: https://chromium-review.googlesource.com/1038793 Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Reviewed-by: Changwan Ryu <changwan@chromium.org> Cr-Commit-Position: refs/heads/master@{#555597} [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapterImpl.java [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java [modify] https://crrev.com/347a2a146b73e8e3f5272705f979917008b324c6/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
,
May 2 2018
Should be fixed.
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/108da5f1453e8226c5c0e5193ddf206c52b14004 commit 108da5f1453e8226c5c0e5193ddf206c52b14004 Author: Andrew Grieve <agrieve@chromium.org> Date: Fri May 11 18:02:43 2018 Android: Remove obsolete proguard -keep for content.browser.input Bug: 636474 Change-Id: I303c9f2f2bd65bf3ba8e7041c72d6ddb75529adf Reviewed-on: https://chromium-review.googlesource.com/1055609 Reviewed-by: Shimi Zhang <ctzsm@chromium.org> Commit-Queue: agrieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/master@{#557930} [modify] https://crrev.com/108da5f1453e8226c5c0e5193ddf206c52b14004/chrome/android/java/proguard.flags |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by yfried...@chromium.org
, Aug 10 2016Status: Assigned (was: Untriaged)