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

Issue 2714863004: [Remote suggestions] Move the throttler into the Scheduler (Closed)

Created:
3 years, 10 months ago by jkrcal
Modified:
3 years, 9 months ago
CC:
chromium-reviews, noyau+watch_chromium.org, ntp-dev+reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Remote suggestions] Move the throttler into the Scheduler This CL moves the throttler into the Scheduler -- which is now the central place to decide whether to and when to fetch. This change also helps in employing I/O scheduler in the Scheduler for prioritization of background fethes. I/O scheduler tasks cannot run on the main thread and thus cannot use PrefService. As throttler depends on prefs, the easiest way is to ask for quota before posting the task to I/O scheduler. This change removes the quota_error fetch status reporting from UMA. This is no problem as all the quota requests are reported to UMA from the throttler anyway. BUG=694325 Review-Url: https://codereview.chromium.org/2714863004 Cr-Commit-Position: refs/heads/master@{#454258} Committed: https://chromium.googlesource.com/chromium/src/+/0a06fdaf8ba732ae795bcb1f2331523babc88dd8

Patch Set 1 #

Total comments: 17

Patch Set 2 : Comments #1 #

Total comments: 1

Patch Set 3 : Rebase #

Patch Set 4 : Ilya's comment #

Messages

Total messages: 18 (7 generated)
jkrcal
Marc, could you PTAL?
3 years, 10 months ago (2017-02-24 09:34:42 UTC) #2
tschumann
https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h File components/ntp_snippets/remote/json_request.h (right): https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h#newcode45 components/ntp_snippets/remote/json_request.h:45: INTERACTIVE_QUOTA_ERROR_OBSOLETE, If it's nothing widely adopted, I'd prefer prefixing ...
3 years, 10 months ago (2017-02-24 10:18:08 UTC) #3
Marc Treib
Thanks for doing this cleanup! Some comments below, but nothing major. https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h File components/ntp_snippets/remote/json_request.h (right): ...
3 years, 10 months ago (2017-02-24 15:50:35 UTC) #4
tschumann
https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h File components/ntp_snippets/remote/json_request.h (right): https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h#newcode45 components/ntp_snippets/remote/json_request.h:45: INTERACTIVE_QUOTA_ERROR_OBSOLETE, On 2017/02/24 15:50:35, Marc Treib wrote: > On ...
3 years, 10 months ago (2017-02-25 17:02:20 UTC) #6
Marc Treib
https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h File components/ntp_snippets/remote/json_request.h (right): https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h#newcode45 components/ntp_snippets/remote/json_request.h:45: INTERACTIVE_QUOTA_ERROR_OBSOLETE, On 2017/02/25 17:02:20, tschumann wrote: > On 2017/02/24 ...
3 years, 9 months ago (2017-02-27 09:18:44 UTC) #7
jkrcal
Thanks for all the comments. PTAL, again! Ilya, could you PTAL at histograms.xml? https://codereview.chromium.org/2714863004/diff/1/components/ntp_snippets/remote/json_request.h File ...
3 years, 9 months ago (2017-02-27 10:26:23 UTC) #9
Marc Treib
lgtm
3 years, 9 months ago (2017-02-27 11:15:14 UTC) #10
Ilya Sherman
histograms.xml lgtm
3 years, 9 months ago (2017-02-27 19:29:59 UTC) #11
Ilya Sherman
By the way, a suggestion for the C++ code (but LGTM either way) https://codereview.chromium.org/2714863004/diff/20001/components/ntp_snippets/remote/json_request.h File ...
3 years, 9 months ago (2017-02-27 19:32:26 UTC) #12
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/2714863004/60001
3 years, 9 months ago (2017-03-02 13:38:52 UTC) #15
commit-bot: I haz the power
3 years, 9 months ago (2017-03-02 15:06:31 UTC) #18
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/0a06fdaf8ba732ae795bcb1f2331...

Powered by Google App Engine
This is Rietveld 408576698