Rename BackgroundBudgetService to BudgetManager

Rename BackgroundBudgetService to BudgetManager

It's clear that there's only going to be a single budget service, so "Background" didn't
make sense anymore. Also, with an interface named BudgetService and a Mojo service named
BudgetService, we needed something other than another BudgetService. The component does
manage calls from the Blink and browser layers into the guts of the BudgetDatabase, so
BudgetManager makes sense.

BUG=617971
Committed: https://crrev.com/bea56c2aeaab7efdbc8dd938b53de48daf5a00cb
Cr-Commit-Position: refs/heads/master@{#412186}

[harkness, 2016-08-12 12:29:52]

harkness@chromium.org changed reviewers:
+ peter@chromium.org

[harkness, 2016-08-12 12:29:52]

There shouldn't be any functional changes, this should just be a rename.

[Peter Beverloo, 2016-08-12 12:39:47]

lgtm % nits

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
File chrome/browser/budget_service/budget_manager.h (right):

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager.h:26: // A budget service to help
Chrome decide how much background work a service
nit: update comment. May also want to s/service worker/origin/g.

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
File chrome/browser/budget_service/budget_manager_unittest.cc (right):

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager_unittest.cc:38: BudgetManager*
GetService() {
nit: GetManager()

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_notification_manager.cc
(right):

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:189: // Get
the budget for the service worker.
nit: origin

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:190:
BudgetManager* service = BudgetManagerFactory::GetForProfile(profile_);
nit: s/service/manager/?

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:290:
BudgetManager* service = BudgetManagerFactory::GetForProfile(profile_);
nit: s/service/manager/

https://codereview.chromium.org/2243813002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2243813002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:45711: +<histogram
name="PushMessaging.BudgetManager">
We've been recording this already, so we can't modify this UMA.

We can either keep it as-is, or create a new one. I don't see a strong reason to
rename it, and I don't feel "BudgetManager" is a clear name for a metric, so I
propose keeping this as-is.

https://codereview.chromium.org/2243813002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:98621: +  <suffix name="BudgetManager"
Since this hasn't ran outside of tests yet, this is fine to rename. Please do
clarify this to the UMA reviewer when sending it out to them.

[harkness, 2016-08-15 08:05:41]

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
File chrome/browser/budget_service/budget_manager.h (right):

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager.h:26: // A budget service to help
Chrome decide how much background work a service
On 2016/08/12 12:39:46, Peter Beverloo wrote:
> nit: update comment. May also want to s/service worker/origin/g.

Done.

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
File chrome/browser/budget_service/budget_manager_unittest.cc (right):

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager_unittest.cc:38: BudgetManager*
GetService() {
On 2016/08/12 12:39:46, Peter Beverloo wrote:
> nit: GetManager()

Done.

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_notification_manager.cc
(right):

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:189: // Get
the budget for the service worker.
On 2016/08/12 12:39:46, Peter Beverloo wrote:
> nit: origin

Done.

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:190:
BudgetManager* service = BudgetManagerFactory::GetForProfile(profile_);
On 2016/08/12 12:39:47, Peter Beverloo wrote:
> nit: s/service/manager/?

Done.

https://codereview.chromium.org/2243813002/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_notification_manager.cc:290:
BudgetManager* service = BudgetManagerFactory::GetForProfile(profile_);
On 2016/08/12 12:39:47, Peter Beverloo wrote:
> nit: s/service/manager/

Done.

https://codereview.chromium.org/2243813002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2243813002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:45711: +<histogram
name="PushMessaging.BudgetManager">
On 2016/08/12 12:39:47, Peter Beverloo wrote:
> We've been recording this already, so we can't modify this UMA.
> 
> We can either keep it as-is, or create a new one. I don't see a strong reason
to
> rename it, and I don't feel "BudgetManager" is a clear name for a metric, so I
> propose keeping this as-is.

Sounds good, I've switched back to the original name.

https://codereview.chromium.org/2243813002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:98621: +  <suffix name="BudgetManager"
On 2016/08/12 12:39:47, Peter Beverloo wrote:
> Since this hasn't ran outside of tests yet, this is fine to rename. Please do
> clarify this to the UMA reviewer when sending it out to them.

Acknowledged.

[harkness, 2016-08-15 08:06:09]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 08:06:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-15 08:58:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-15 08:58:47]

Dry run: This issue passed the CQ dry run.

[harkness, 2016-08-15 10:06:21]

The CQ bit was checked by harkness@chromium.org

[harkness, 2016-08-15 10:06:21]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2243813002/#ps20001
(title: "code review cleanup")

[commit-bot: I haz the power, 2016-08-15 10:06:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-15 10:09:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-15 10:09:39]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[harkness, 2016-08-15 10:17:02]

harkness@chromium.org changed reviewers:
+ bauerb@chromium.org, holte@chromium.org

[harkness, 2016-08-15 10:17:03]

@holte, could you take a look at histograms.xml please?

@bauerb, could you take a look at chrome/browser/prefs/browser_prefs.cc please?

https://codereview.chromium.org/2243813002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2243813002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:98621: -  <suffix
name="BackgroundBudgetService"
The code that calls this database UMA is only invoked in tests right now, so
changing this UMA should be ok.

[Bernhard Bauer, 2016-08-15 11:53:22]

prefs LGTM

[Steven Holte, 2016-08-15 21:57:31]

lgtm

https://codereview.chromium.org/2243813002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2243813002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:98621: -  <suffix
name="BackgroundBudgetService"
On 2016/08/15 10:17:02, harkness wrote:
> The code that calls this database UMA is only invoked in tests right now, so
> changing this UMA should be ok.

Acknowledged.

[harkness, 2016-08-16 07:18:58]

The CQ bit was checked by harkness@chromium.org

[commit-bot: I haz the power, 2016-08-16 07:19:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 07:23:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-16 07:26:06]

Description was changed from

==========
Rename BackgroundBudgetService to BudgetManager

It's clear that there's only going to be a single budget service, so
"Background" didn't
make sense anymore. Also, with an interface named BudgetService and a Mojo
service named
BudgetService, we needed something other than another BudgetService. The
component does
manage calls from the Blink and browser layers into the guts of the
BudgetDatabase, so
BudgetManager makes sense.

BUG=617971
==========

to

==========
Rename BackgroundBudgetService to BudgetManager

It's clear that there's only going to be a single budget service, so
"Background" didn't
make sense anymore. Also, with an interface named BudgetService and a Mojo
service named
BudgetService, we needed something other than another BudgetService. The
component does
manage calls from the Blink and browser layers into the guts of the
BudgetDatabase, so
BudgetManager makes sense.

BUG=617971

Committed: https://crrev.com/bea56c2aeaab7efdbc8dd938b53de48daf5a00cb
Cr-Commit-Position: refs/heads/master@{#412186}
==========

[commit-bot: I haz the power, 2016-08-16 07:26:07]

Patchset 2 (id:??) landed as
https://crrev.com/bea56c2aeaab7efdbc8dd938b53de48daf5a00cb
Cr-Commit-Position: refs/heads/master@{#412186}