Make all quota-exceeding messages in the storage API explain what the failure

Make all quota-exceeding messages in the storage API explain what the failure
is. This involves adding descriptions to all errors from ExtensionsQuotaService,
affecting the other APIs that use this (bookmarks and webRequest).

BUG=144533
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154415

[not at google - send to devlin, 2012-08-23 06:40:43]

sky: bookmarks API changes
mpcomplete: everything else

[sky, 2012-08-23 15:57:05]

Matt should review the whole thing, the change to c/t and c/b/b LGTM

[Matt Perry, 2012-08-23 18:30:35]

Please file a BUG and link it in the CL description. Then LGTM with nits below.

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/web_request/web_request_api.cc (right):

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/web_request/web_request_api.cc:1551: :
QuotaLimitHeuristic(config, map, "clear cache"),
This won't make sense to the extension author. Let's go with
"handlerBehaviorChanged"

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
File chrome/browser/extensions/settings/settings_api.cc (right):

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
chrome/browser/extensions/settings/settings_api.cc:159:
"MAX_WRITE_OPERATIONS_PER_HOUR"));
Let's use a consistent naming scheme for these errors. The bookmarks and
webRequest APIs use "multi word phrases" while this uses "ALL_CAPS_CONSTANTS". I
slightly prefer the former.

[not at google - send to devlin, 2012-08-23 23:46:41]

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
File chrome/browser/extensions/settings/settings_api.cc (right):

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
chrome/browser/extensions/settings/settings_api.cc:159:
"MAX_WRITE_OPERATIONS_PER_HOUR"));
On 2012/08/23 18:30:35, Matt Perry wrote:
> Let's use a consistent naming scheme for these errors. The bookmarks and
> webRequest APIs use "multi word phrases" while this uses "ALL_CAPS_CONSTANTS".
I
> slightly prefer the former.

I also prefer to read the multi-word phrases; however, I did it like this here
because these constants are actually named like this in the API schema
(api::storage::sync::MAX_WRITE_OPERATIONS_PER_MINUTE, thank you
json-schema-compiler). And I also like that if developers search for that string
in the docs it'll come up like that.

Consistency is king, though... and ideally all APIs that use these constants
should have them defined in the JSON; that helps developers understand the
errors... then we can give them their symbolic names?

Do you agree? If so, what is the best course of action... do that in a
follow-up, or do that now (happy either way)? Or just change these to be
phrases?

[Matt Perry, 2012-08-23 23:55:05]

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
File chrome/browser/extensions/settings/settings_api.cc (right):

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
chrome/browser/extensions/settings/settings_api.cc:159:
"MAX_WRITE_OPERATIONS_PER_HOUR"));
On 2012/08/23 23:46:41, kalman wrote:
> On 2012/08/23 18:30:35, Matt Perry wrote:
> > Let's use a consistent naming scheme for these errors. The bookmarks and
> > webRequest APIs use "multi word phrases" while this uses
"ALL_CAPS_CONSTANTS".
> I
> > slightly prefer the former.
> 
> I also prefer to read the multi-word phrases; however, I did it like this here
> because these constants are actually named like this in the API schema
> (api::storage::sync::MAX_WRITE_OPERATIONS_PER_MINUTE, thank you
> json-schema-compiler). And I also like that if developers search for that
string
> in the docs it'll come up like that.
> 
> Consistency is king, though... and ideally all APIs that use these constants
> should have them defined in the JSON; that helps developers understand the
> errors... then we can give them their symbolic names?
> 
> Do you agree? If so, what is the best course of action... do that in a
> follow-up, or do that now (happy either way)? Or just change these to be
> phrases?

That makes sense. Use SYMBOLIC_NAMES everywhere now, and add the definitions to
the schema files (can be done in a followup).

[not at google - send to devlin, 2012-08-24 04:45:04]

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/web_request/web_request_api.cc (right):

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/web_request/web_request_api.cc:1551: :
QuotaLimitHeuristic(config, map, "clear cache"),
On 2012/08/23 18:30:35, Matt Perry wrote:
> This won't make sense to the extension author. Let's go with
> "handlerBehaviorChanged"

Changed to "MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES" as kind-of
discussed, does this sound ok?

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
File chrome/browser/extensions/settings/settings_api.cc (right):

http://codereview.chromium.org/10871034/diff/1/chrome/browser/extensions/sett...
chrome/browser/extensions/settings/settings_api.cc:159:
"MAX_WRITE_OPERATIONS_PER_HOUR"));
On 2012/08/23 23:55:06, Matt Perry wrote:
> On 2012/08/23 23:46:41, kalman wrote:
> > On 2012/08/23 18:30:35, Matt Perry wrote:
> > > Let's use a consistent naming scheme for these errors. The bookmarks and
> > > webRequest APIs use "multi word phrases" while this uses
> "ALL_CAPS_CONSTANTS".
> > I
> > > slightly prefer the former.
> > 
> > I also prefer to read the multi-word phrases; however, I did it like this
here
> > because these constants are actually named like this in the API schema
> > (api::storage::sync::MAX_WRITE_OPERATIONS_PER_MINUTE, thank you
> > json-schema-compiler). And I also like that if developers search for that
> string
> > in the docs it'll come up like that.
> > 
> > Consistency is king, though... and ideally all APIs that use these constants
> > should have them defined in the JSON; that helps developers understand the
> > errors... then we can give them their symbolic names?
> > 
> > Do you agree? If so, what is the best course of action... do that in a
> > follow-up, or do that now (happy either way)? Or just change these to be
> > phrases?
> 
> That makes sense. Use SYMBOLIC_NAMES everywhere now, and add the definitions
to
> the schema files (can be done in a followup).

I've done it in this change because I was unsure what to call them, and putting
them in the documentation now will help with that.

So, done. Could you sanity check the names?

http://codereview.chromium.org/10871034/diff/3003/chrome/common/extensions/do...
File chrome/common/extensions/docs/css/ApiRefStyles.css (right):

http://codereview.chromium.org/10871034/diff/3003/chrome/common/extensions/do...
chrome/common/extensions/docs/css/ApiRefStyles.css:335: word-break: break-all;
without this the long property names overflow :\ I can't think of a better
solution, overflow:hidden also looks bad. Perhaps ending in elipses, but that's
something the new docs server would be better at.

That, or have more concise names. Hm.

[not at google - send to devlin, 2012-08-28 04:10:50]

ping

[Aaron Boodman, 2012-08-28 05:22:10]

Matt's out on vacation for a week. I'll take a look.

[Aaron Boodman, 2012-08-28 05:29:22]

LGTM

[commit-bot: I haz the power, 2012-08-28 05:30:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/19

[commit-bot: I haz the power, 2012-08-28 05:30:47]

Presubmit check for 10871034-19 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright (\(c\) )?(2012|2011|2010|2009|2008|2007|2006|2006-2008) The
Chromium Authors\. All rights reserved\.\n.*? Use of this source code is
governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?:
\*/)?\n
Found a bad license header in these files:
  chrome/common/extensions/docs/css/ApiRefStyles.css

Presubmit checks took 27.5s to calculate.

[not at google - send to devlin, 2012-08-28 05:31:32]

On 2012/08/28 05:30:47, I haz the power (commit-bot) wrote:
> Presubmit check for 10871034-19 failed and returned exit status 1.
> 
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Warnings **
> License must match:
> .*? Copyright (\(c\) )?(2012|2011|2010|2009|2008|2007|2006|2006-2008) The
> Chromium Authors\. All rights reserved\.\n.*? Use of this source code is
> governed by a BSD-style license that can be\n.*? found in the LICENSE
file\.(?:
> \*/)?\n
> Found a bad license header in these files:
>   chrome/common/extensions/docs/css/ApiRefStyles.css
> 
> Presubmit checks took 27.5s to calculate.

bleh

[commit-bot: I haz the power, 2012-08-28 06:56:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/16002

[commit-bot: I haz the power, 2012-08-28 08:40:26]

Try job failure for 10871034-16002 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-08-29 08:01:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/16005

[commit-bot: I haz the power, 2012-08-29 09:29:38]

Try job failure for 10871034-16005 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-08-30 01:52:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/24005

[commit-bot: I haz the power, 2012-08-30 06:40:24]

Try job failure for 10871034-24005 (retry) (retry) on linux_chromeos for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-08-30 07:11:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/24005

[commit-bot: I haz the power, 2012-08-30 08:39:06]

Try job failure for 10871034-24005 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-30 09:25:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/24005

[commit-bot: I haz the power, 2012-08-30 11:22:46]

Try job failure for 10871034-24005 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-31 00:56:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/24005

[commit-bot: I haz the power, 2012-08-31 03:06:11]

Try job failure for 10871034-24005 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-31 03:09:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/24005

[commit-bot: I haz the power, 2012-08-31 05:12:06]

Try job failure for 10871034-24005 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-31 05:15:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/10871034/24005

[commit-bot: I haz the power, 2012-08-31 09:45:31]

Change committed as 154415