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

Issue 1308723005: Popular sites on the NTP: re-download popular suggestions once per Chrome run (Closed)

Created:
5 years, 3 months ago by Marc Treib
Modified:
5 years, 3 months ago
Reviewers:
Bernhard Bauer, battre
CC:
chromium-reviews, cbentzel+watch_chromium.org, pam+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@knn_ordering
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Popular sites on the NTP: re-download popular suggestions once per Chrome run BUG=516618 Committed: https://crrev.com/f79ed40d0ed19927d312d80a4bf797ab143bc025 Cr-Commit-Position: refs/heads/master@{#350144}

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Total comments: 6

Patch Set 4 : review #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -24 lines) Patch
M chrome/browser/android/most_visited_sites.cc View 1 2 3 3 chunks +33 lines, -11 lines 0 comments Download
M chrome/browser/android/popular_sites.cc View 1 1 chunk +5 lines, -1 line 0 comments Download
M chrome/browser/net/file_downloader.h View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/net/file_downloader.cc View 1 2 3 3 chunks +16 lines, -9 lines 0 comments Download
M chrome/browser/supervised_user/supervised_user_service.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 20 (7 generated)
Marc Treib
Next part :) (based on knn's pending CL, and will need to be rebased on ...
5 years, 3 months ago (2015-09-08 14:26:35 UTC) #2
Bernhard Bauer
lgtm
5 years, 3 months ago (2015-09-08 16:06:59 UTC) #3
Marc Treib
+battre for c/b/net changes. PTAL!
5 years, 3 months ago (2015-09-09 08:11:05 UTC) #5
Marc Treib
On 2015/09/09 08:11:05, Marc Treib wrote: > +battre for c/b/net changes. PTAL! Ping!
5 years, 3 months ago (2015-09-11 16:15:02 UTC) #7
battre
What do you think of adding a unit test? https://codereview.chromium.org/1308723005/diff/60001/chrome/browser/android/most_visited_sites.cc File chrome/browser/android/most_visited_sites.cc (right): https://codereview.chromium.org/1308723005/diff/60001/chrome/browser/android/most_visited_sites.cc#newcode137 chrome/browser/android/most_visited_sites.cc:137: ...
5 years, 3 months ago (2015-09-14 14:31:25 UTC) #8
Marc Treib
Immediate comments addressed. Agreed that a unit test for FileDownloader would make sense. Can I ...
5 years, 3 months ago (2015-09-18 11:44:24 UTC) #9
Marc Treib
Ping! Is it okay to land this as-is, and add the test in a followup? ...
5 years, 3 months ago (2015-09-22 08:58:02 UTC) #10
battre
lgtm
5 years, 3 months ago (2015-09-22 09:07:57 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308723005/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308723005/100001
5 years, 3 months ago (2015-09-22 09:46:28 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/106081)
5 years, 3 months ago (2015-09-22 10:53:14 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308723005/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308723005/100001
5 years, 3 months ago (2015-09-22 10:57:11 UTC) #18
commit-bot: I haz the power
Committed patchset #5 (id:100001)
5 years, 3 months ago (2015-09-22 12:38:16 UTC) #19
commit-bot: I haz the power
5 years, 3 months ago (2015-09-22 12:40:24 UTC) #20
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/f79ed40d0ed19927d312d80a4bf797ab143bc025
Cr-Commit-Position: refs/heads/master@{#350144}

Powered by Google App Engine
This is Rietveld 408576698