Sanitize PrefStore interface.

chrome/common/pref_store.h

 // Copyright (c) 2010 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.
 
 #ifndef CHROME_COMMON_PREF_STORE_H_
 #define CHROME_COMMON_PREF_STORE_H_
 #pragma once
 
 #include <string>
 
 #include "base/basictypes.h"
 
-class DictionaryValue;
 class Value;
 
 // This is an abstract interface for reading and writing from/to a persistent
 // preference store, used by PrefService. An implementation using a JSON file
 // can be found in JsonPrefStore, while an implementation without any backing
 // store for testing can be found in TestingPrefStore. Furthermore, there is
 // CommandLinePrefStore, which bridges command line options to preferences and
 // ConfigurationPolicyPrefStore, which is used for hooking up configuration
 // policy with the preference subsystem.
 class PrefStore {
  public:
   // Observer interface for monitoring PrefStore.
   class ObserverInterface {

[battre (please use the other), 2010/12/08 12:24:15]

Observer instead of ObserverInterface?

grep -r Observer chrome/ | wc -l
5231
grep -r ObserverInterface chrome/ | wc -l
0

[Mattias Nissler (ping if slow), 2010/12/09 10:20:20]

Well, fight with Danno over it, I don't really care.
Done.

    public:
     virtual ~ObserverInterface() {}
 
     // Called when the value for the given |key| in the store changes.
     virtual void OnPrefValueChanged(const std::string& key) = 0;
     // Notification about the PrefStore being fully initialized.
     virtual void OnInitializationCompleted() = 0;
   };
 
-  // Unique integer code for each type of error so we can report them
-  // distinctly in a histogram.
-  // NOTE: Don't change the order here as it will change the server's meaning
-  // of the histogram.
-  enum PrefReadError {
-    PREF_READ_ERROR_NONE = 0,
-    PREF_READ_ERROR_JSON_PARSE,
-    PREF_READ_ERROR_JSON_TYPE,
-    PREF_READ_ERROR_ACCESS_DENIED,
-    PREF_READ_ERROR_FILE_OTHER,
-    PREF_READ_ERROR_FILE_LOCKED,
-    PREF_READ_ERROR_NO_FILE,
-    PREF_READ_ERROR_JSON_REPEAT,
-    PREF_READ_ERROR_OTHER,
-    PREF_READ_ERROR_FILE_NOT_SPECIFIED
+  // Return values for GetValue().
+  enum ReadResult {
+    // Value found and returned.
+    READ_OK,
+    // No value present, but skip other pref stores and use default.

[battre (please use the other), 2010/12/08 12:24:15]

I don't understand the "but skip other pref stores" part. Is this an instruction
to the caller? How would this be decided? Is this part of a policy that says
"use this value / use no value (override by NULL) / use what ever somebody
defines"?

[Mattias Nissler (ping if slow), 2010/12/09 10:20:20]

This kludge is here for proxy prefs and tells the PrefValueStore to return the
default value. So it is an instruction for the caller. It's up to the
implementor of the interface to decide when it wants to return this.

+    READ_USE_DEFAULT,
+    // No value present.
+    READ_NO_VALUE,
   };
 
-  // To require that the default value be used for a preference, a
-  // PrefStore can set the value in its own prefs dictionary to the
-  // sentinel Value returned by this function.
-  // TODO(danno): Instead of having a sentinel value, pref stores
-  // should return a richer set of information from the property
-  // accessor methods to indicate that the default should be used.
-  static Value* CreateUseDefaultSentinelValue();
-
-  // Returns true if a value is the special sentinel value created by
-  // CreateUseDefaultSentinelValue.
-  static bool IsUseDefaultSentinelValue(Value* value);
-
   PrefStore() {}
   virtual ~PrefStore() {}
 
   // Add and remove observers.
   virtual void AddObserver(ObserverInterface* observer) {}
   virtual void RemoveObserver(ObserverInterface* observer) {}
 
   // Whether the store has completed all asynchronous initialization.
   virtual bool IsInitializationComplete() { return true; }
 
-  // Whether the store is in a pseudo-read-only mode where changes are not
-  // actually persisted to disk.  This happens in some cases when there are
-  // read errors during startup.
-  virtual bool ReadOnly() const { return true; }
-
-  // TODO(danno): PrefValueStore shouldn't allow direct access to the
-  // DictionaryValue. Instead, it should have getters that return a
-  // richer set of information for a pref, including if the store
-  // wants to return the default value for a preference.
-  virtual DictionaryValue* prefs() const = 0;
-
-  virtual PrefReadError ReadPrefs() = 0;
-
-  virtual bool WritePrefs() { return true; }
-
-  virtual void ScheduleWritePrefs() { }
+  // Get the value for a given preference |key| and stores it in |result|.
+  // Ownership remains with the PrefStore.

[battre (please use the other), 2010/12/08 12:24:15]

Does not modify |result| if the key does not exist.

[Mattias Nissler (ping if slow), 2010/12/09 10:20:20]

Done.

+  virtual ReadResult GetValue(const std::string& key, Value** result) const = 0;
 
   DISALLOW_COPY_AND_ASSIGN(PrefStore);
 };
 
 #endif  // CHROME_COMMON_PREF_STORE_H_