refactor autocomplete logic from omnibox |
||
Issue descriptionSince we want to try different autocomplete view logic for issue 539536 , we need to refactor UrlBar.java as follows: 1) Separate AutocompleteEditText (name not finalized) from UrlBar and have UrlBar inherit from AutocompleteEditText. 2) Separate the core logic inside AutocompleteEditText from View related logic in a way that we can pick out the right model based on flag value at native initialization.
,
May 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/912333cc6c89529bee8dc2167c2b9602f76fda6c commit 912333cc6c89529bee8dc2167c2b9602f76fda6c Author: changwan <changwan@chromium.org> Date: Wed May 24 16:31:38 2017 Fix AutocompleteEditTextTest Since dummy input connection was used, beginBatchEdit() and endBatchEdit() was not calling TextView#onBeginBatchEdit() correctly. We can use the TextView's EditableInputConnection and expect correct results. BUG= 722908 Review-Url: https://codereview.chromium.org/2903633004 Cr-Commit-Position: refs/heads/master@{#474321} [modify] https://crrev.com/912333cc6c89529bee8dc2167c2b9602f76fda6c/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java
,
Jun 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a5770d08b1a9c1c7157934f512714195396057fe commit a5770d08b1a9c1c7157934f512714195396057fe Author: changwan <changwan@chromium.org> Date: Fri Jun 09 02:50:19 2017 Move batchedit related logics into InputConnection This helps reduce the number of API calls between View related logic and autocomplete logic, and thus makes the split easier. Also removes mention of URL bar from comments in AutocompleteEditText.java. BUG= 722908 Review-Url: https://codereview.chromium.org/2896143002 Cr-Commit-Position: refs/heads/master@{#478173} [modify] https://crrev.com/a5770d08b1a9c1c7157934f512714195396057fe/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java [modify] https://crrev.com/a5770d08b1a9c1c7157934f512714195396057fe/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/UrlBarTest.java
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/199ca72e5d3dc9a48514b390b58933c515c46b77 commit 199ca72e5d3dc9a48514b390b58933c515c46b77 Author: Changwan Ryu <changwan@chromium.org> Date: Fri Jun 16 22:49:12 2017 Refactor autocomplete text logic into AutocompleteTextModel class This refactoring is needed to try out different models. In doing so, - moved most logic into model class. - moved paste related logic from UrlBar into AutocompleteEditTextModel such that shouldAutocomplete() becomes more self-contained. - moved most of the logging into model. - added new call paths into AutocompleteEditTextTest. There is no production logic change except: - added additional null checks in case the methods get called from view constructor. which should not have any adverse effect. BUG= 722908 Change-Id: Id238df597ed8bdc6d07f14faa97386d237b3217c Reviewed-on: https://chromium-review.googlesource.com/532239 Commit-Queue: Changwan Ryu <changwan@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#480204} [modify] https://crrev.com/199ca72e5d3dc9a48514b390b58933c515c46b77/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java [add] https://crrev.com/199ca72e5d3dc9a48514b390b58933c515c46b77/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModel.java [add] https://crrev.com/199ca72e5d3dc9a48514b390b58933c515c46b77/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java [modify] https://crrev.com/199ca72e5d3dc9a48514b390b58933c515c46b77/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java [modify] https://crrev.com/199ca72e5d3dc9a48514b390b58933c515c46b77/chrome/android/java_sources.gni [modify] https://crrev.com/199ca72e5d3dc9a48514b390b58933c515c46b77/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java
,
Jun 16 2017
,
Jun 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44c78922bc7c54229051765e159ff3069aaa1814 commit 44c78922bc7c54229051765e159ff3069aaa1814 Author: Changwan Ryu <changwan@chromium.org> Date: Thu Jun 29 17:17:18 2017 Switch back to null check in AutocompleteEditText In https://chromium-review.googlesource.com/c/543307/, it was found that ensureModel() approach ends up switching back and forth between the two models if we have to read a feature flag to determine the model type. Normally, the call order is as follows: 1) View#View() -> Calls internal methods that refers to mModel. 2) onCreateInputConnection() -> super.onCreateInputConnection() returns null. 3) onNativeLibraryReady() -> Feature flag becomes available. 4) onCreateInputConnection() -> User focused omnibox, and super.onCreateInputConnection() returns a non-null value. After this point we cannot swap models. It is theoretically possible that non-null InputConnection is created before 3) (e.g., if we autofocus), but in practice it is very unlikely and we can ignore for the upcoming feature experiment. The interactions can be simplified if we create a model in the first non-null onCreateInputConnection(), by which point we know what model type we need to create. In doing so, we cache mLastEditWasPaste and mIgnoreTextChangesForAutocomplete such that they can be removed from ModelBase interface and feed the values when we create the model. Out of the two values, mLastEditWasPaste won't be needed in the new model. BUG= 539536 , 722908 Change-Id: I269b5dce8d10ecd30aaca5737dac3e3a302f0d61 Reviewed-on: https://chromium-review.googlesource.com/553724 Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Changwan Ryu <changwan@chromium.org> Cr-Commit-Position: refs/heads/master@{#483405} [modify] https://crrev.com/44c78922bc7c54229051765e159ff3069aaa1814/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java [modify] https://crrev.com/44c78922bc7c54229051765e159ff3069aaa1814/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModel.java [modify] https://crrev.com/44c78922bc7c54229051765e159ff3069aaa1814/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java [modify] https://crrev.com/44c78922bc7c54229051765e159ff3069aaa1814/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java [modify] https://crrev.com/44c78922bc7c54229051765e159ff3069aaa1814/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java [modify] https://crrev.com/44c78922bc7c54229051765e159ff3069aaa1814/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java |
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, May 18 2017