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

Issue 10802063: Fix two bugs in the toast (Closed)

Created:
8 years, 5 months ago by cpu_(ooo_6.6-7.5)
Modified:
8 years, 5 months ago
Reviewers:
Finnur, Mark Larson
CC:
chromium-reviews, grt+watch_chromium.org
Visibility:
Public.

Description

Fix two bugs in the toast 1- The US locale is sometimes "en" and sometimes "en-US" 2- Users that hit "don't bug me" where shown the same toast again when we ran the toast twice in a row. Now we compare the prefix of the experiment witht the 'client' prefix if equal it means we already toasted this user with this same experiment. BUG=130169 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147972

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -10 lines) Patch
M chrome/installer/util/google_chrome_distribution.cc View 3 chunks +22 lines, -10 lines 2 comments Download

Messages

Total messages: 4 (0 generated)
cpu_(ooo_6.6-7.5)
please review at your earliest chance.
8 years, 5 months ago (2012-07-22 20:58:07 UTC) #1
Mark Larson
On 2012/07/22 20:58:07, cpu wrote: > please review at your earliest chance. LGTM
8 years, 5 months ago (2012-07-22 21:18:45 UTC) #2
cpu_(ooo_6.6-7.5)
I checked in as-is but I still need finnur's feedback. I will create another CL ...
8 years, 5 months ago (2012-07-23 22:36:08 UTC) #3
Finnur
8 years, 5 months ago (2012-07-25 10:51:10 UTC) #4
http://codereview.chromium.org/10802063/diff/1/chrome/installer/util/google_c...
File chrome/installer/util/google_chrome_distribution.cc (right):

http://codereview.chromium.org/10802063/diff/1/chrome/installer/util/google_c...
chrome/installer/util/google_chrome_distribution.cc:761: if (base_group ==
client.substr(0, 2)) {
nit: Probably more readable to combine these into one if:

if (client.size() > 2 && base_group == client.substr(0, 2)) {
...
}

http://codereview.chromium.org/10802063/diff/1/chrome/installer/util/google_c...
chrome/installer/util/google_chrome_distribution.cc:763: return;
Are you intentionally setting no results for Omaha to pick up and report back?
In other words, shouldn't you be using SetClient with a specific results code,
like we do on lines 782, 787, 793 (to name a few)?

Powered by Google App Engine
This is Rietveld 408576698