Extract PrefServiceBase into chrome/browser/api. Use in api and autofill.

chrome/browser/api/prefs/pref_member.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/api/prefs/pref_member.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/value_conversions.h"
-#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/api/prefs/pref_service_base.h"
 #include "chrome/common/chrome_notification_types.h"
 
 using content::BrowserThread;
 
 namespace subtle {
 
 PrefMemberBase::PrefMemberBase()
     : observer_(NULL),
       prefs_(NULL),
       setting_value_(false) {
 }
 
 PrefMemberBase::~PrefMemberBase() {
   Destroy();
 }
 
 
 void PrefMemberBase::Init(const char* pref_name,
-                          PrefService* prefs,
+                          PrefServiceBase* prefs,
                           content::NotificationObserver* observer) {
   DCHECK(pref_name);
   DCHECK(prefs);
   DCHECK(pref_name_.empty());  // Check that Init is only called once.
   observer_ = observer;
   prefs_ = prefs;
   pref_name_ = pref_name;
   // Check that the preference is registered.
-  DCHECK(prefs_->FindPreference(pref_name_.c_str()))
+  DCHECK(prefs_->GetUserPrefValue(pref_name_.c_str()))

[Mattias Nissler (ping if slow), 2012/08/16 15:16:42]

Wait, what's this? This is incorrect.

[Jói, 2012/08/16 15:34:18]

See below.

       << pref_name << " not registered.";
 
   // Add ourselves as a pref observer so we can keep our local value in sync.
   prefs_->AddPrefObserver(pref_name, this);
 }
 
 void PrefMemberBase::Destroy() {
   if (prefs_ && !pref_name_.empty()) {
     prefs_->RemovePrefObserver(pref_name_.c_str(), this);
     prefs_ = NULL;
@@ skipping 13 matching lines @@
                              const content::NotificationDetails& details) {
   VerifyValuePrefName();
   DCHECK(chrome::NOTIFICATION_PREF_CHANGED == type);
   UpdateValueFromPref();
   if (!setting_value_ && observer_)
     observer_->Observe(type, source, details);
 }
 
 void PrefMemberBase::UpdateValueFromPref() const {
   VerifyValuePrefName();
-  const PrefService::Preference* pref =
-      prefs_->FindPreference(pref_name_.c_str());
-  DCHECK(pref);
   if (!internal())
     CreateInternal();
-  internal()->UpdateValue(pref->GetValue()->DeepCopy(), pref->IsManaged());
+  internal()->UpdateValue(
+      prefs_->GetUserPrefValue(pref_name_.c_str())->DeepCopy(),

[Mattias Nissler (ping if slow), 2012/08/16 15:16:42]

also incorrect

[Jói, 2012/08/16 15:34:18]

This, and the thing above, was me trying to get rid of FindPreference from the
base API.  I misread the GetUserPrefValue documentation, and I see now that this
is not equivalent.

I wonder if I should instead simply add GetPrefValue(name) which returns a
base::Value* like these two guys, but basically just does
FindPreference(name)->GetValue().  I think that would make both places
equivalent, although the DCHECK at the top would be checking a different thing
it should be semantically the same?

[Mattias Nissler (ping if slow), 2012/08/16 16:48:33]

You could indeed make a GetValue() function on PrefService (it's OK to expose
that), and then call that from Preference::GetValue(). Then again, I would
consider PrefService::Preference to be part of the generic interface actually,
so I'd feel better if that piece would move to PrefServiceBase as well.

+      prefs_->IsManagedPreference(pref_name_.c_str()));
 }
 
 void PrefMemberBase::VerifyPref() const {
   VerifyValuePrefName();
   if (!internal())
     UpdateValueFromPref();
 }
 
 PrefMemberBase::Internal::Internal()
     : thread_id_(BrowserThread::UI),
@@ skipping 75 matching lines @@
 template <>
 void PrefMember<FilePath>::UpdatePref(const FilePath& value) {
   prefs()->SetFilePath(pref_name().c_str(), value);
 }
 
 template <>
 bool PrefMember<FilePath>::Internal::UpdateValueInternal(const Value& value)
     const {
   return base::GetValueAsFilePath(value, &value_);
 }