Extension Settings API: make it so that when leveldb storage areas fail to be

Extension Settings API: make it so that when leveldb storage areas fail to be
opened/created, all operations fail from then on rather than falling back to an
in-memory implementation.

Slight refactor to SettingsFrontend/Backend to inject a factory for creating
SettingsStorage objects, for testing when storage area construction fails.


BUG=103514
TEST=*ExtensionSettings*

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

[not at google - send to devlin, 2011-11-10 06:59:33]

(Also: re-add "Extensions" back to a bunch of tests that it got accidentally
removed from in the settings rename I did yesterday).

@aa this is the first of several changes I'll be making to fix up the error
handling of the settings API, as we discussed a while ago.

@mpcomplete: you might want to look at that bug (I just CC'ed you on it, it's
also mentioned in the BUG= line) for what I'm going to do here.

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

http://codereview.chromium.org/8497065/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_api.cc:40: bool success =
RunWithStorage(observers.get(), storage);
(storage can't be NULL)

[Matt Perry, 2011-11-11 00:49:38]

lgtm

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
File chrome/browser/extensions/settings/settings_backend.cc (right):

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
chrome/browser/extensions/settings/settings_backend.cc:77: // It's fine to
create the quota enforcer underneath the sync later, since
later = layer?

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
File chrome/browser/extensions/settings/settings_backend.h (right):

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
chrome/browser/extensions/settings/settings_backend.h:76:
SettingsStorageFactory* const storage_factory_;
Is there a reason this is const? This will just prevent you from pointing
storage_factory_ at a different object. Did you mean "const
SettingsStorageFactory*" (which will prevent you from modifying the data of the
object being pointed to)?

[not at google - send to devlin, 2011-11-12 02:10:27]

Oops forgot to send out the comments for this.

(Already submitted, sending out for the record...)

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
File chrome/browser/extensions/settings/settings_backend.cc (right):

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
chrome/browser/extensions/settings/settings_backend.cc:77: // It's fine to
create the quota enforcer underneath the sync later, since
On 2011/11/11 00:49:38, Matt Perry wrote:
> later = layer?

Done.

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
File chrome/browser/extensions/settings/settings_backend.h (right):

http://codereview.chromium.org/8497065/diff/6001/chrome/browser/extensions/se...
chrome/browser/extensions/settings/settings_backend.h:76:
SettingsStorageFactory* const storage_factory_;
On 2011/11/11 00:49:38, Matt Perry wrote:
> Is there a reason this is const? This will just prevent you from pointing
> storage_factory_ at a different object. Did you mean "const
> SettingsStorageFactory*" (which will prevent you from modifying the data of
the
> object being pointed to)?

Yep that was deliberate.  It could be "const SettingsStoargeFactory* const
storage_factory_" but "const SettingsStorageFactory" isn't really that
interesting, it would mean I'd need to add const to the ::Create methods, which
is just boring.

I... like to add const to pointers in the same way that I like putting final to
members in Java... just documentation that this won't change.