New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 783995 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
(OOO slow)
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[TTS] ML Retap: Tap on a short word selects when quickly following a nearby word

Project Member Reported by donnd@google.com, Nov 10 2017

Issue description

With the short-word-suppression Ranker model we're seeing the ability to tap-select a short word only when done right after a tap-select on a regular word nearby.

We need to investigate whether this is a CS problem or in Ranker -- I'll take a look.
 

Comment 1 by donnd@google.com, Nov 16 2017

I'm pretty sure this bug is in Contextual Search.  It's location-related, not timing related.  When tapping on a word nearby when the Bar is showing we don't close and reopen the Bar, we just update the search.  I suspect the code-flow is updating the existing example in this case adding a second set of datapoints to the proto.

Comment 2 by donnd@google.com, Nov 30 2017

The problem is that the Search is still in progress and the Bar still active when the retap is processed.  We need to detect this and log the outcomes and reset the logger in order to separate the features of the previous gesture from the new gesture.

Comment 3 by donnd@google.com, Dec 1 2017

Summary: [TTS] ML Retap: Tap on a short word selects when quickly following a nearby word (was: [TTS] Tap on a short word selects when quickly following a nearby word)

Comment 5 by donnd@google.com, Dec 5 2017

Labels: -M-64 M-65
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bd4f41d9b265249e14854b715c91032d4a94211

commit 5bd4f41d9b265249e14854b715c91032d4a94211
Author: Donn Denman <donnd@google.com>
Date: Fri Dec 15 20:41:51 2017

[TTS] Fix ML bug: tap near previous selection, initialization.

Fixes a problem with ML not being applied correctly on a "retap", a tap
near the previous selection.  The problem is that the Search is still in
progress and the Bar still active when the retap is processed.

Adds a new Internal State TAP_GESTURE_COMMIT that allows code to run
during the initial stage of tap gesture processing.  This new state is
used to handle the retap by detecting if the panel is still open.

Loading the Ranker predictor is now moved to an earlier stage of tap
processing (using the new internal state) to allow it to be fully loaded
and ready to predict by the time all the prediction features have been
gathered.

Refactored writing the outcomes to Ranker in
ContextualSearchPanelMetrics so it can be called when appropriate.  Now
logging outcomes and resetting the CSRankerLogger is done whenever the
UI is hidden or a retap is detected.

BUG= 783995 

Change-Id: I3122a20200696682205379fdd619e655262520f9
Reviewed-on: https://chromium-review.googlesource.com/802540
Commit-Queue: Donn Denman <donnd@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524452}
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchHeuristics.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateController.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateHandler.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapSuppressionHeuristics.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFakeServer.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateControllerWrapper.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/5bd4f41d9b265249e14854b715c91032d4a94211/chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateTest.java

Comment 7 by donnd@google.com, Dec 15 2017

Status: Fixed (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9537a64495887e20dc5e2bf04543a58b8d841ef8

commit 9537a64495887e20dc5e2bf04543a58b8d841ef8
Author: Donn Denman <donnd@chromium.org>
Date: Tue Dec 19 02:07:32 2017

Revert "[TTS] Fix ML bug: tap near previous selection, initialization."

This reverts commit 5bd4f41d9b265249e14854b715c91032d4a94211.

Reason for revert: Suspected in Crash in issue 795936.

Original change's description:
> [TTS] Fix ML bug: tap near previous selection, initialization.
>
> Fixes a problem with ML not being applied correctly on a "retap", a tap
> near the previous selection.  The problem is that the Search is still in
> progress and the Bar still active when the retap is processed.
>
> Adds a new Internal State TAP_GESTURE_COMMIT that allows code to run
> during the initial stage of tap gesture processing.  This new state is
> used to handle the retap by detecting if the panel is still open.
>
> Loading the Ranker predictor is now moved to an earlier stage of tap
> processing (using the new internal state) to allow it to be fully loaded
> and ready to predict by the time all the prediction features have been
> gathered.
>
> Refactored writing the outcomes to Ranker in
> ContextualSearchPanelMetrics so it can be called when appropriate.  Now
> logging outcomes and resetting the CSRankerLogger is done whenever the
> UI is hidden or a retap is detected.
>
> BUG= 783995 
>
> Change-Id: I3122a20200696682205379fdd619e655262520f9
> Reviewed-on: https://chromium-review.googlesource.com/802540
> Commit-Queue: Donn Denman <donnd@chromium.org>
> Reviewed-by: Theresa <twellington@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524452}

TBR=donnd@chromium.org,twellington@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  783995 ,795936
Change-Id: Ia98480ec8d825877a12b283a7dccc4f4947c0291
Reviewed-on: https://chromium-review.googlesource.com/833310
Commit-Queue: Donn Denman <donnd@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524912}
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchHeuristics.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateController.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateHandler.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapSuppressionHeuristics.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFakeServer.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateControllerWrapper.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/9537a64495887e20dc5e2bf04543a58b8d841ef8/chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateTest.java

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 19 2017

Labels: merge-merged-3299
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d33aff13df31c695c7d8be7f8d4b48182220010

commit 5d33aff13df31c695c7d8be7f8d4b48182220010
Author: Donn Denman <donnd@chromium.org>
Date: Tue Dec 19 17:01:27 2017

Revert "[TTS] Fix ML bug: tap near previous selection, initialization."

This reverts commit 5bd4f41d9b265249e14854b715c91032d4a94211.

Reason for revert: Suspected in Crash in issue 795936.

Original change's description:
> [TTS] Fix ML bug: tap near previous selection, initialization.
>
> Fixes a problem with ML not being applied correctly on a "retap", a tap
> near the previous selection.  The problem is that the Search is still in
> progress and the Bar still active when the retap is processed.
>
> Adds a new Internal State TAP_GESTURE_COMMIT that allows code to run
> during the initial stage of tap gesture processing.  This new state is
> used to handle the retap by detecting if the panel is still open.
>
> Loading the Ranker predictor is now moved to an earlier stage of tap
> processing (using the new internal state) to allow it to be fully loaded
> and ready to predict by the time all the prediction features have been
> gathered.
>
> Refactored writing the outcomes to Ranker in
> ContextualSearchPanelMetrics so it can be called when appropriate.  Now
> logging outcomes and resetting the CSRankerLogger is done whenever the
> UI is hidden or a retap is detected.
>
> BUG= 783995 
>
> Change-Id: I3122a20200696682205379fdd619e655262520f9
> Reviewed-on: https://chromium-review.googlesource.com/802540
> Commit-Queue: Donn Denman <donnd@chromium.org>
> Reviewed-by: Theresa <twellington@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524452}

TBR=donnd@chromium.org,twellington@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  783995 ,795936
Change-Id: Ia98480ec8d825877a12b283a7dccc4f4947c0291
Reviewed-on: https://chromium-review.googlesource.com/833310
Commit-Queue: Donn Denman <donnd@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#524912}(cherry picked from commit 9537a64495887e20dc5e2bf04543a58b8d841ef8)
Reviewed-on: https://chromium-review.googlesource.com/834108
Cr-Commit-Position: refs/branch-heads/3299@{#3}
Cr-Branched-From: bdab2c10eaad8f821b6eac3c083da4f5fb4c20c8-refs/heads/master@{#524906}
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchHeuristics.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateController.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateHandler.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapSuppressionHeuristics.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFakeServer.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateControllerWrapper.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/5d33aff13df31c695c7d8be7f8d4b48182220010/chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateTest.java

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25453694ec4f6c1f45965c0b1f6a7354d4b7d568

commit 25453694ec4f6c1f45965c0b1f6a7354d4b7d568
Author: Donn Denman <donnd@google.com>
Date: Thu Jan 04 20:09:38 2018

Reland "[TTS] Fix ML bug: tap near previous selection, initialization."

This is a reland of 5bd4f41d9b265249e14854b715c91032d4a94211
Original change's description:
> [TTS] Fix ML bug: tap near previous selection, initialization.
> 
> Fixes a problem with ML not being applied correctly on a "retap", a tap
> near the previous selection.  The problem is that the Search is still in
> progress and the Bar still active when the retap is processed.
> 
> Adds a new Internal State TAP_GESTURE_COMMIT that allows code to run
> during the initial stage of tap gesture processing.  This new state is
> used to handle the retap by detecting if the panel is still open.
> 
> Loading the Ranker predictor is now moved to an earlier stage of tap
> processing (using the new internal state) to allow it to be fully loaded
> and ready to predict by the time all the prediction features have been
> gathered.
> 
> Refactored writing the outcomes to Ranker in
> ContextualSearchPanelMetrics so it can be called when appropriate.  Now
> logging outcomes and resetting the CSRankerLogger is done whenever the
> UI is hidden or a retap is detected.
> 
> BUG= 783995 
> 
> Change-Id: I3122a20200696682205379fdd619e655262520f9
> Reviewed-on: https://chromium-review.googlesource.com/802540
> Commit-Queue: Donn Denman <donnd@chromium.org>
> Reviewed-by: Theresa <twellington@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524452}

Bug:  783995 
Change-Id: I3f224ecc53cd18d84d1a8e97316245b817d5f4db
Reviewed-on: https://chromium-review.googlesource.com/841244
Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527076}
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchHeuristics.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateController.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateHandler.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/TapSuppressionHeuristics.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFakeServer.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateControllerWrapper.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/25453694ec4f6c1f45965c0b1f6a7354d4b7d568/chrome/android/junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchInternalStateTest.java

Sign in to add a comment