Propagate more information about ValueStore errors to callers, notably an

Propagate more information about ValueStore errors to callers, notably an
error code in addition to the error message. This turned out to be a
surprisingly invasive yak shave and is accompanied by a few very small fixes
to inefficient and overly complex code I noticed along the way.

This is the first stage in making the chrome.storage extension/app API able to
gracefully recover from leveldb corruption.

BUG=261623
R=mpcomplete@chromium.org
TBR=tim@chromium.org
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222178

[not at google - send to devlin, 2013-09-06 01:55:17]

Hey Matt, hopefully I didn't get too carried away here (I tried very hard not
to). There were a couple of non-trivial changes in there, and more than a few
changes to leveldb_value_store. I also tried very hard not to add any extra
features that need testing - that will come in the next patch (when I propagate
write errors to Clear() and implement recovery from sync).

[Matt Perry, 2013-09-06 20:07:29]

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/storage/storage_api.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/storage/storage_api.cc:84: SetResult(result);
You have a PassError - how about a PassSettings?

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/le...
File chrome/browser/value_store/leveldb_value_store.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/le...
chrome/browser/value_store/leveldb_value_store.cc:26: // Hack for converting a
base::FilePath to a std::string for use by leveldb.
FilePath already has an AsUTF8Unsafe

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/va...
File chrome/browser/value_store/value_store_frontend_unittest.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/va...
chrome/browser/value_store/value_store_frontend_unittest.cc:96:
//ResetStorage();
?

[not at google - send to devlin, 2013-09-06 21:44:28]

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/storage/storage_api.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/storage/storage_api.cc:84: SetResult(result);
On 2013/09/06 20:07:30, Matt Perry wrote:
> You have a PassError - how about a PassSettings?

I haven't need it yet - mostly because DictionaryValue has a swap() method,
which Error doesn't have. I can add it for consistency if you like.

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/le...
File chrome/browser/value_store/leveldb_value_store.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/le...
chrome/browser/value_store/leveldb_value_store.cc:26: // Hack for converting a
base::FilePath to a std::string for use by leveldb.
On 2013/09/06 20:07:30, Matt Perry wrote:
> FilePath already has an AsUTF8Unsafe

Done.

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/va...
File chrome/browser/value_store/value_store_frontend_unittest.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/value_store/va...
chrome/browser/value_store/value_store_frontend_unittest.cc:96:
//ResetStorage();
On 2013/09/06 20:07:30, Matt Perry wrote:
> ?

oh oops. Fixed.

[Matt Perry, 2013-09-06 21:50:37]

lgtm

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/storage/storage_api.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/storage/storage_api.cc:84: SetResult(result);
On 2013/09/06 21:44:28, kalman wrote:
> On 2013/09/06 20:07:30, Matt Perry wrote:
> > You have a PassError - how about a PassSettings?
> 
> I haven't need it yet - mostly because DictionaryValue has a swap() method,
> which Error doesn't have. I can add it for consistency if you like.

Not a big deal, but it does seem idiomatically clearer.

[not at google - send to devlin, 2013-09-10 00:21:32]

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/storage/storage_api.cc (right):

https://codereview.chromium.org/24021002/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/storage/storage_api.cc:84: SetResult(result);
On 2013/09/06 21:50:37, Matt Perry wrote:
> On 2013/09/06 21:44:28, kalman wrote:
> > On 2013/09/06 20:07:30, Matt Perry wrote:
> > > You have a PassError - how about a PassSettings?
> > 
> > I haven't need it yet - mostly because DictionaryValue has a swap() method,
> > which Error doesn't have. I can add it for consistency if you like.
> 
> Not a big deal, but it does seem idiomatically clearer.

Done.

[commit-bot: I haz the power, 2013-09-10 00:23:04]

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

[commit-bot: I haz the power, 2013-09-10 00:54:11]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-10 01:21:29]

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

[commit-bot: I haz the power, 2013-09-10 01:31:30]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-10 01:52:39]

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

[commit-bot: I haz the power, 2013-09-10 01:54:34]

Change committed as 222178