Add a Restore() method to ValueStore and make StorageAPI use it

chrome/browser/extensions/api/storage/policy_value_store.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/storage/policy_value_store.h"
 
 #include "base/logging.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/value_store/value_store_change.h"
@@ skipping 143 matching lines @@
 
 ValueStore::WriteResult PolicyValueStore::Remove(
     const std::vector<std::string>& keys) {
   return MakeWriteResult(ReadOnlyError(util::NoKey()));
 }
 
 ValueStore::WriteResult PolicyValueStore::Clear() {
   return MakeWriteResult(ReadOnlyError(util::NoKey()));
 }
 
+bool PolicyValueStore::Restore() {
+  return delegate_->Restore();

[not at google - send to devlin, 2014/02/14 19:35:56]

We should be able to restore from policy?

+joaodasilva

[Devlin, 2014/02/18 23:55:22]

I think so, yes.  The Policy stores, even though they're read-only, are backed
by a normal store.  If they get corrupted (which will hopefully be less likely,
but could still happen in weird cases, like the windows restore bug), I think we
still need the ability to wipe them.

But I'll defer to Joao's knowledge, here.

[Joao da Silva, 2014/02/19 08:32:03]

This is fine. The PolicyValueStore is used to detect policy changes across
browser restarts, and contains a copy of the policies loaded by Chrome.

So SetCurrentPolicy() will overwrite the whole thing again with sane values as
soon as Chrome reloads the policies again, or at the next restart.

[not at google - send to devlin, 2014/02/19 17:55:59]

Ok. So there's no reason to restore them now? Devlin could you add a comment
here to that effect?

[Joao da Silva, 2014/02/19 18:05:43]

I don't know how LevelDB handles corrupted databases. Will it recover if the
corrupted values are written again? Or are there circumstances where those
writes would fail? Could the database fail to open on a subsequent restart?

I'd keep the Restore() calls to the delegate to be on the safe side given my
ignorance of how LevelDB works. You know the database better, so I defer a
decision to you :-)

[not at google - send to devlin, 2014/02/19 18:13:53]

Restore on the leveldb instance is really just force-clear. To actually restore
data properly it will need to pull state from policy after restore.

So I think eventually you will need to pull from policy on restore, ideally.
Same for RestoreKey. Otherwise it will give spurious change events on startup.

However the same can be said for sync, and I commented there that we might want
to just do that in a follow up.

+}
+
+bool PolicyValueStore::RestoreKey(const std::string& key) {
+  return delegate_->RestoreKey(key);
+}
+
 }  // namespace extensions