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

Issue 2726413002: Reland - Omnibox - Zero Suggest - Log When/Whether Contextual Search is Possible (Closed)

Created:
3 years, 9 months ago by Mark P
Modified:
3 years, 9 months ago
Reviewers:
Peter Kasting
CC:
chromium-reviews, jdonnelly+watch_chromium.org, asvitkine+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland - Omnibox - Zero Suggest - Log When/Whether Contextual Search is Possible Reland https://codereview.chromium.org/2724303002 after changing one static initializer from a std::string to a const char[]. See original changelist description for more details: --- This will allow us to estimate the "headroom", i.e., how many people and how many focus events could we actually help with contextual suggestions. No functional changes (intended). TEST=using about:histograms --- TBR=pkasting TBR=isherman BUG=696077 Review-Url: https://codereview.chromium.org/2726413002 Cr-Commit-Position: refs/heads/master@{#454676} Committed: https://chromium.googlesource.com/chromium/src/+/3d89cdcd61c6b95d514e2c9ed86e929483e0cc76

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+155 lines, -24 lines) Patch
M components/omnibox/browser/zero_suggest_provider.h View 1 chunk +6 lines, -0 lines 0 comments Download
M components/omnibox/browser/zero_suggest_provider.cc View 7 chunks +81 lines, -24 lines 3 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +68 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (9 generated)
Mark P
Hey Peter, take a look at this one-line change if you want before I land ...
3 years, 9 months ago (2017-03-03 19:05:55 UTC) #5
Peter Kasting
On 2017/03/03 19:05:55, Mark P wrote: > Hey Peter, take a look at this one-line ...
3 years, 9 months ago (2017-03-03 20:09:11 UTC) #6
Mark P
https://codereview.chromium.org/2726413002/diff/1/components/omnibox/browser/zero_suggest_provider.cc File components/omnibox/browser/zero_suggest_provider.cc (right): https://codereview.chromium.org/2726413002/diff/1/components/omnibox/browser/zero_suggest_provider.cc#newcode84 components/omnibox/browser/zero_suggest_provider.cc:84: const char kArbitraryInsecureUrlString[] = "http://www.google.com/"; This line changed from ...
3 years, 9 months ago (2017-03-03 20:14:29 UTC) #9
Peter Kasting
LGTM https://codereview.chromium.org/2726413002/diff/1/components/omnibox/browser/zero_suggest_provider.cc File components/omnibox/browser/zero_suggest_provider.cc (right): https://codereview.chromium.org/2726413002/diff/1/components/omnibox/browser/zero_suggest_provider.cc#newcode84 components/omnibox/browser/zero_suggest_provider.cc:84: const char kArbitraryInsecureUrlString[] = "http://www.google.com/"; On 2017/03/03 20:14:29, ...
3 years, 9 months ago (2017-03-03 20:21:56 UTC) #10
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/2726413002/1
3 years, 9 months ago (2017-03-03 21:10:30 UTC) #12
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/3d89cdcd61c6b95d514e2c9ed86e929483e0cc76
3 years, 9 months ago (2017-03-03 21:16:56 UTC) #15
Mark P
On 2017/03/03 21:16:56, commit-bot: I haz the power wrote: > Committed patchset #1 (id:1) as ...
3 years, 9 months ago (2017-03-03 21:21:37 UTC) #16
Mark P
3 years, 9 months ago (2017-03-03 22:12:40 UTC) #17
Message was sent while issue was closed.
Did cleanup in
https://codereview.chromium.org/2725333004/

--mark

https://codereview.chromium.org/2726413002/diff/1/components/omnibox/browser/...
File components/omnibox/browser/zero_suggest_provider.cc (right):

https://codereview.chromium.org/2726413002/diff/1/components/omnibox/browser/...
components/omnibox/browser/zero_suggest_provider.cc:84: const char
kArbitraryInsecureUrlString[] = "http://www.google.com/";
On 2017/03/03 20:21:55, Peter Kasting wrote:
> On 2017/03/03 20:14:29, Mark P wrote:
> > This line changed from
> > std::string kArbitraryInsecureUrlString = "http://www.google.com/";
> > to its current state.
> 
> Cool.
> 
> Prefer constexpr here.

Done.

Powered by Google App Engine
This is Rietveld 408576698