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

Issue 2875553002: [TTS] Remove the blacklist and first char metrics. (Closed)

Created:
3 years, 7 months ago by Donn Denman
Modified:
3 years, 7 months ago
Reviewers:
Theresa, Steven Holte
CC:
chromium-reviews, twellington+watch_chromium.org, mdjones+watch_chromium.org, donnd+watch_chromium.org, asvitkine+watch_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[TTS] Remove the blacklist and first char metrics. We were recording the first character of the selection to use as a hint for triggering. We don't currently have permission to send this to Ranker so it's being removed for now, and the histogram being marked obsolete. We don't yet have permission to send any content signals to Ranker so removing some promising ones like word-length from the send-to-ranker list but keeping the underlying data collection for now. Remove the entire blacklist and obsoleting the associated histogram. BUG=720193 Review-Url: https://codereview.chromium.org/2875553002 Cr-Commit-Position: refs/heads/master@{#471058} Committed: https://chromium.googlesource.com/chromium/src/+/ad9a27a4d180efbc526c04545e9eea31f8dcecb7

Patch Set 1 #

Total comments: 6

Patch Set 2 : Removed and obsoleted the started-with-capital uma and histogram. Plus a rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -321 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java View 8 chunks +0 lines, -38 lines 0 comments Download
D chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchBlacklist.java View 1 chunk +0 lines, -223 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java View 1 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLogger.java View 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java View 2 chunks +0 lines, -14 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionHandler.java View 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchUma.java View 1 3 chunks +0 lines, -26 lines 0 comments Download
M chrome/android/java_sources.gni View 1 1 chunk +0 lines, -1 line 0 comments Download
M tools/metrics/histograms/enums.xml View 1 1 chunk +3 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (11 generated)
Donn Denman
Theresa, PTAL. Thanks!
3 years, 7 months ago (2017-05-10 18:33:20 UTC) #6
Theresa
https://codereview.chromium.org/2875553002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java File chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java (left): https://codereview.chromium.org/2875553002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java#oldcode136 chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java:136: ContextualSearchUma.logStartedWithCapitalResultsSeen(mWasSearchContentViewSeen); This method still needs to be removed from ...
3 years, 7 months ago (2017-05-10 18:47:09 UTC) #7
Donn Denman
Theresa, PTAL. Thanks! https://codereview.chromium.org/2875553002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java File chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java (left): https://codereview.chromium.org/2875553002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java#oldcode136 chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanelMetrics.java:136: ContextualSearchUma.logStartedWithCapitalResultsSeen(mWasSearchContentViewSeen); On 2017/05/10 18:47:09, Theresa wrote: ...
3 years, 7 months ago (2017-05-10 21:28:55 UTC) #10
Theresa
lgtm
3 years, 7 months ago (2017-05-10 21:39:45 UTC) #11
Donn Denman
Steven, PTAL at histograms.xml. Thanks!
3 years, 7 months ago (2017-05-10 21:42:33 UTC) #13
Steven Holte
lgtm
3 years, 7 months ago (2017-05-11 17:40:21 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2875553002/20001
3 years, 7 months ago (2017-05-11 17:42:06 UTC) #16
commit-bot: I haz the power
3 years, 7 months ago (2017-05-11 20:35:50 UTC) #19
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ad9a27a4d180efbc526c04545e9e...

Powered by Google App Engine
This is Rietveld 408576698