Plumb reserve method of the BudgetAPI

Plumb reserve method of the BudgetAPI

This CL adds the reserve method to both the budget_api mojo service and
to the BudgetService web platform API. This also provides a skeleton
implementation of the mojo service in chrome/browser, but the full
connection is left for a future CL. There are new LayoutTests and an
update to the BudgetService Javascript mock to test the new reserve
functionality.

This does provide some TODO cleanup, adding a success parameter to mojo
operations that could fail due to database errors, adding mappings from
strings to mojo values, and updating the LayoutTests to be more dynamic.

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

[harkness, 2016-09-05 16:06:31]

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

[Peter Beverloo, 2016-09-05 17:06:17]

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

https://codereview.chromium.org/2309863002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager.cc:112: // No default case.
micro nit: it feels to me like this is indented two spaces too much

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

https://codereview.chromium.org/2309863002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_service_impl.cc:50:
callback.Run(blink::mojom::BudgetServiceErrorType::NO_ERROR, false);
This is missing a TODO?

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js
(right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:27:
this.cost = new Object();
this.cost_ = {};

(underscore suffix to indicate that it's private; we want a dictionary here, so
easy initialisation will do.)

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:28:
this.budget = new Array();
this.budget_ = [];

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:29:
this.error = budgetService.BudgetServiceErrorType.NO_ERROR;
this.error_ = budgetService....

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:39: //
mapping is needed to mirror the mapping that happens in the mojo layer.
micro nit: mojo -> Mojo

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:39: //
mapping is needed to mirror the mapping that happens in the mojo layer.
My gut feel is that adding some comments above the methods indicating whether
they (a) implement a Mojo function, or (b) exist for testing purposes would
increase clarity a lot.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:41: var
mojoOperationType = budgetService.BudgetOperationType.INVALID_OPERATION;
nit: var -> let

(Try to avoid "var" everywhere. Use "let" when you have to change the value, or
"const" when you don't.)

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:54:
this.budget.push({time:addTime, budget_at:addBudget});
nit: this indentation is inconsistent with the value you create on line 49

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:19:
mojom::blink::BudgetOperationType stringToOperationType(const AtomicString&
operation)
nit: please don't indent this code by an additional four characters.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:36: }
Please add a NOTREACHED() at the end of this method.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:64:
resolver->reject(DOMException::create(NotSupportedError, "Invalid operation type
specified"));
nit: the clean way of doing this (here and on lines 98 and 123) would be to
*not* create the promise and the resolver yourself, but rather bail out early
through:

return ScriptPromise::rejectWithDOMException(..);

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:120: 
nit: mind adding a TODO that we need to do a secure origin check here?

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/public/p...
File
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom
(right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom:18:
};
nit: mind documenting what these all mean? Being somewhat verbose in these IDL
files is good.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom:19:

nit: no double blank lines

[haraken, 2016-09-06 01:46:43]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-09-06 01:46:44]

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:118: if (!context)

Nit: I don't think this should happen.

[harkness, 2016-09-06 15:45:01]

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

https://codereview.chromium.org/2309863002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager.cc:112: // No default case.
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> micro nit: it feels to me like this is indented two spaces too much

Done.

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

https://codereview.chromium.org/2309863002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_service_impl.cc:50:
callback.Run(blink::mojom::BudgetServiceErrorType::NO_ERROR, false);
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> This is missing a TODO?

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js
(right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:27:
this.cost = new Object();
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> this.cost_ = {};
> 
> (underscore suffix to indicate that it's private; we want a dictionary here,
so
> easy initialisation will do.)

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:28:
this.budget = new Array();
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> this.budget_ = [];

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:29:
this.error = budgetService.BudgetServiceErrorType.NO_ERROR;
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> this.error_ = budgetService....

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:39: //
mapping is needed to mirror the mapping that happens in the mojo layer.
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> micro nit: mojo -> Mojo

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:39: //
mapping is needed to mirror the mapping that happens in the mojo layer.
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> My gut feel is that adding some comments above the methods indicating whether
> they (a) implement a Mojo function, or (b) exist for testing purposes would
> increase clarity a lot.

I added in a lot of comments, possibly too many. Take a look and let me know
what you think.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:41: var
mojoOperationType = budgetService.BudgetOperationType.INVALID_OPERATION;
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: var -> let
> 
> (Try to avoid "var" everywhere. Use "let" when you have to change the value,
or
> "const" when you don't.)

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:54:
this.budget.push({time:addTime, budget_at:addBudget});
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: this indentation is inconsistent with the value you create on line 49

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:19:
mojom::blink::BudgetOperationType stringToOperationType(const AtomicString&
operation)
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: please don't indent this code by an additional four characters.

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:36: }
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> Please add a NOTREACHED() at the end of this method.

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:64:
resolver->reject(DOMException::create(NotSupportedError, "Invalid operation type
specified"));
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: the clean way of doing this (here and on lines 98 and 123) would be to
> *not* create the promise and the resolver yourself, but rather bail out early
> through:
> 
> return ScriptPromise::rejectWithDOMException(..);

I updated here and 123, but 98 is a different situation, because it's on the
return path so the resolver/promise have already been created. Is that still a
valid place for the rejectWithDOMException syntax?

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:118: if (!context)
On 2016/09/06 01:46:44, haraken wrote:
> 
> Nit: I don't think this should happen.

In a previous review, you mentioned that it could be a problem if the script was
being executed on a detached frame. Peter explained a bit of the world of
execution contexts and how some of the underlying code is changing to deal with
the possibility of null contexts. Is this something that has changed so that
it's now safe?

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:120: 
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: mind adding a TODO that we need to do a secure origin check here?

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/public/p...
File
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom
(right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom:18:
};
On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: mind documenting what these all mean? Being somewhat verbose in these IDL
> files is good.

Done.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom:19:

On 2016/09/05 17:06:16, Peter Beverloo wrote:
> nit: no double blank lines

Done.

[harkness, 2016-09-06 15:49:00]

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

[harkness, 2016-09-06 15:49:01]

+tsepez Could you please take a look at the updated budget_service.mojom?

[Peter Beverloo, 2016-09-06 15:54:10]

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

https://codereview.chromium.org/2309863002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager.cc:112: // No default case.
On 2016/09/06 15:45:00, harkness wrote:
> On 2016/09/05 17:06:16, Peter Beverloo wrote:
> > micro nit: it feels to me like this is indented two spaces too much
> 
> Done.

No you didn't?

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:19:
mojom::blink::BudgetOperationType stringToOperationType(const AtomicString&
operation)
On 2016/09/06 15:45:01, harkness wrote:
> On 2016/09/05 17:06:16, Peter Beverloo wrote:
> > nit: please don't indent this code by an additional four characters.
> 
> Done.

This isn't done either.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:64:
resolver->reject(DOMException::create(NotSupportedError, "Invalid operation type
specified"));
On 2016/09/06 15:45:01, harkness wrote:
> On 2016/09/05 17:06:16, Peter Beverloo wrote:
> > nit: the clean way of doing this (here and on lines 98 and 123) would be to
> > *not* create the promise and the resolver yourself, but rather bail out
early
> > through:
> > 
> > return ScriptPromise::rejectWithDOMException(..);
> 
> I updated here and 123, but 98 is a different situation, because it's on the
> return path so the resolver/promise have already been created. Is that still a
> valid place for the rejectWithDOMException syntax?

98's fine, thanks :)

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js
(right):

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:38: //
This is called directly from test javascript to set up the return value
OCD: JavaScript

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom
(right):

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom:38:
Reserve(url.mojom.Origin origin, BudgetOperationType operation) =>
(BudgetServiceErrorType errorType, bool success);
errorType -> error_type (you use hacker_style on line 26 already)

[harkness, 2016-09-06 16:17:26]

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

https://codereview.chromium.org/2309863002/diff/1/chrome/browser/budget_servi...
chrome/browser/budget_service/budget_manager.cc:112: // No default case.
On 2016/09/06 15:54:10, Peter Beverloo wrote:
> On 2016/09/06 15:45:00, harkness wrote:
> > On 2016/09/05 17:06:16, Peter Beverloo wrote:
> > > micro nit: it feels to me like this is indented two spaces too much
> > 
> > Done.
> 
> No you didn't?

Alas, git cl format changed it back after I changed it, and this directory
requires git cl format.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:19:
mojom::blink::BudgetOperationType stringToOperationType(const AtomicString&
operation)
On 2016/09/06 15:54:10, Peter Beverloo wrote:
> On 2016/09/06 15:45:01, harkness wrote:
> > On 2016/09/05 17:06:16, Peter Beverloo wrote:
> > > nit: please don't indent this code by an additional four characters.
> > 
> > Done.
> 
> This isn't done either.

git cl format strikes again, but if I override it, presubmit complains. I'm
going to leave the indentation broken, and let you follow up with the tools
author.

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:64:
resolver->reject(DOMException::create(NotSupportedError, "Invalid operation type
specified"));
On 2016/09/06 15:54:10, Peter Beverloo wrote:
> On 2016/09/06 15:45:01, harkness wrote:
> > On 2016/09/05 17:06:16, Peter Beverloo wrote:
> > > nit: the clean way of doing this (here and on lines 98 and 123) would be
to
> > > *not* create the promise and the resolver yourself, but rather bail out
> early
> > > through:
> > > 
> > > return ScriptPromise::rejectWithDOMException(..);
> > 
> > I updated here and 123, but 98 is a different situation, because it's on the
> > return path so the resolver/promise have already been created. Is that still
a
> > valid place for the rejectWithDOMException syntax?
> 
> 98's fine, thanks :)

Acknowledged.

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js
(right):

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/budget/budget-service-mock.js:38: //
This is called directly from test javascript to set up the return value
On 2016/09/06 15:54:10, Peter Beverloo wrote:
> OCD: JavaScript

Done.

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom
(right):

https://codereview.chromium.org/2309863002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/budget_service/budget_service.mojom:38:
Reserve(url.mojom.Origin origin, BudgetOperationType operation) =>
(BudgetServiceErrorType errorType, bool success);
On 2016/09/06 15:54:10, Peter Beverloo wrote:
> errorType -> error_type (you use hacker_style on line 26 already)

Done.

[Tom Sepez, 2016-09-06 17:58:46]

mojom LGTM

[Peter Beverloo, 2016-09-06 18:02:22]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2016-09-06 18:02:22]

lgtm

[commit-bot: I haz the power, 2016-09-06 18:03:03]

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-09-06 18:40:51]

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

[commit-bot: I haz the power, 2016-09-06 18:40:52]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[haraken, 2016-09-07 01:55:47]

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:118: if (!context)
On 2016/09/06 15:45:01, harkness wrote:
> On 2016/09/06 01:46:44, haraken wrote:
> > 
> > Nit: I don't think this should happen.
> 
> In a previous review, you mentioned that it could be a problem if the script
was
> being executed on a detached frame. Peter explained a bit of the world of
> execution contexts and how some of the underlying code is changing to deal
with
> the possibility of null contexts. Is this something that has changed so that
> it's now safe?

Maybe are you talking about other getExecutionContext()?

scriptState->getExecutionContext() returns an ExecutionContext even after the
associated frame is detached. If you want to prevent the following code from
being run after the frame detach, you need to add the following check:

  if (!m_scriptState->contextIsValid())
    return;

Either way, the real problem is that APIs around getting contexts are confusing.
The binding team will take care of it.

[harkness, 2016-09-07 09:24:52]

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/budget/BudgetService.cpp (right):

https://codereview.chromium.org/2309863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/budget/BudgetService.cpp:118: if (!context)
On 2016/09/07 01:55:47, haraken wrote:
> On 2016/09/06 15:45:01, harkness wrote:
> > On 2016/09/06 01:46:44, haraken wrote:
> > > 
> > > Nit: I don't think this should happen.
> > 
> > In a previous review, you mentioned that it could be a problem if the script
> was
> > being executed on a detached frame. Peter explained a bit of the world of
> > execution contexts and how some of the underlying code is changing to deal
> with
> > the possibility of null contexts. Is this something that has changed so that
> > it's now safe?
> 
> Maybe are you talking about other getExecutionContext()?
> 
> scriptState->getExecutionContext() returns an ExecutionContext even after the
> associated frame is detached. If you want to prevent the following code from
> being run after the frame detach, you need to add the following check:
> 
>   if (!m_scriptState->contextIsValid())
>     return;
> 
> Either way, the real problem is that APIs around getting contexts are
confusing.
> The binding team will take care of it.

Thanks very much for the explanation, I've removed the null check.

[harkness, 2016-09-07 09:26:59]

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

[commit-bot: I haz the power, 2016-09-07 09:27:10]

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-09-07 10:57:23]

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

[commit-bot: I haz the power, 2016-09-07 10:57:24]

Dry run: This issue passed the CQ dry run.

[harkness, 2016-09-07 11:02:39]

The CQ bit was checked by harkness@chromium.org

[harkness, 2016-09-07 11:02:39]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2309863002/#ps80001
(title: "Rename BudgetServiceErrorType::NO_ERROR to ::NONE to avoid Windows
constant name clash.")

[commit-bot: I haz the power, 2016-09-07 11:02:51]

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-09-07 11:07:07]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-07 11:09:09]

Description was changed from

==========
Plumb reserve method of the BudgetAPI

This CL adds the reserve method to both the budget_api mojo service and
to the BudgetService web platform API. This also provides a skeleton
implementation of the mojo service in chrome/browser, but the full
connection is left for a future CL. There are new LayoutTests and an
update to the BudgetService Javascript mock to test the new reserve
functionality.

This does provide some TODO cleanup, adding a success parameter to mojo
operations that could fail due to database errors, adding mappings from
strings to mojo values, and updating the LayoutTests to be more dynamic.

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

to

==========
Plumb reserve method of the BudgetAPI

This CL adds the reserve method to both the budget_api mojo service and
to the BudgetService web platform API. This also provides a skeleton
implementation of the mojo service in chrome/browser, but the full
connection is left for a future CL. There are new LayoutTests and an
update to the BudgetService Javascript mock to test the new reserve
functionality.

This does provide some TODO cleanup, adding a success parameter to mojo
operations that could fail due to database errors, adding mappings from
strings to mojo values, and updating the LayoutTests to be more dynamic.

BUG=617971

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

[commit-bot: I haz the power, 2016-09-07 11:09:10]

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