Move PromoResourceService from Profile to BrowserProcessImpl/local_state().

chrome/browser/web_resource/notification_promo.cc

Index: chrome/browser/web_resource/notification_promo.cc
diff --git a/chrome/browser/web_resource/notification_promo.cc b/chrome/browser/web_resource/notification_promo.cc
index 692d305233835fd197ddc18b0cfb9250ebe989ff..1aeeb3836179d9066c36d3cdd14c009e52f6f71b 100644
--- a/chrome/browser/web_resource/notification_promo.cc
+++ b/chrome/browser/web_resource/notification_promo.cc
@@ -12,10 +12,13 @@
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
+#include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #include "base/values.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/prefs/pref_service.h"
-#include "chrome/browser/profiles/profile_impl.h"
+#include "chrome/browser/prefs/pref_service_simple.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/web_resource/promo_resource_service.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/net/url_util.h"
@@ -53,7 +56,6 @@ const char kPrefPromoMaxViews[] = "max_views";
 const char kPrefPromoGroup[] = "group";
 const char kPrefPromoViews[] = "views";
 const char kPrefPromoClosed[] = "closed";
-const char kPrefPromoGPlusRequired[] = "gplus_required";
 
 // Returns a string suitable for the Promo Server URL 'osname' value.
 std::string PlatformString() {
@@ -189,9 +191,8 @@ void AppendQueryParameter(GURL* url,
 
 }  // namespace
 
-NotificationPromo::NotificationPromo(Profile* profile)
-    : profile_(profile),
-      prefs_(profile_->GetPrefs()),
+NotificationPromo::NotificationPromo()
+    : prefs_(g_browser_process->local_state()),

[Robert Sesek, 2013/01/03 21:40:10]

Can you dependency inject this, potentially allowing you to remove the
ForTesting methods?

[Dan Beam, 2013/01/03 22:47:38]

I was doing that before but estade@ and I thought a little awkwardness for
testing was an acceptable tradeoff to remove this from all the calling code's
API.

We talked about this over chat and TestingBrowserProcess::SetLocalState() seemed
to do the trick.

       promo_type_(NO_PROMO),
       promo_payload_(new base::DictionaryValue()),
       start_(0.0),
@@ -205,9 +206,7 @@ NotificationPromo::NotificationPromo(Profile* profile)
       group_(0),
       views_(0),
       closed_(false),
-      gplus_required_(false),
       new_notification_(false) {
-  DCHECK(profile);
   DCHECK(prefs_);
 }
 
@@ -271,9 +270,6 @@ void NotificationPromo::InitFromJson(const DictionaryValue& json,
   // Payload.
   const DictionaryValue* payload = NULL;
   if (promo->GetDictionary("payload", &payload)) {
-    payload->GetBoolean("gplus_required", &gplus_required_);
-    DVLOG(1) << "gplus_required_ = " << gplus_required_;
-
     base::Value* ppcopy = DeepCopyAndResolveStrings(payload, strings);
     DCHECK(ppcopy && ppcopy->IsType(base::Value::TYPE_DICTIONARY));
     promo_payload_.reset(static_cast<base::DictionaryValue*>(ppcopy));
@@ -296,7 +292,7 @@ void NotificationPromo::InitFromJson(const DictionaryValue& json,
 }
 
 void NotificationPromo::CheckForNewNotification() {
-  NotificationPromo old_promo(profile_);
+  NotificationPromo old_promo(prefs_);
   old_promo.InitFromPrefs(promo_type_);
   const double old_start = old_promo.start_;
   const double old_end = old_promo.end_;
@@ -316,10 +312,16 @@ void NotificationPromo::OnNewNotification() {
 }
 
 // static
+void NotificationPromo::RegisterPrefs(PrefServiceSimple* local_state) {
+  local_state->RegisterDictionaryPref(kPrefPromoObject);
+}
+
+// static
 void NotificationPromo::RegisterUserPrefs(PrefServiceSyncable* prefs) {
+  // TODO(dbeam): Remove in M28 when we're reasonably sure all prefs are gone.
   prefs->RegisterDictionaryPref(kPrefPromoObject,
-                                new base::DictionaryValue,
                                 PrefServiceSyncable::UNSYNCABLE_PREF);
+  prefs->ClearPref(kPrefPromoObject);
 }
 
 void NotificationPromo::WritePrefs() {
@@ -341,8 +343,6 @@ void NotificationPromo::WritePrefs() {
   ntp_promo->SetInteger(kPrefPromoViews, views_);
   ntp_promo->SetBoolean(kPrefPromoClosed, closed_);
 
-  ntp_promo->SetBoolean(kPrefPromoGPlusRequired, gplus_required_);
-
   base::ListValue* promo_list = new base::ListValue;
   promo_list->Set(0, ntp_promo);  // Only support 1 promo for now.
 
@@ -389,8 +389,6 @@ void NotificationPromo::InitFromPrefs(PromoType promo_type) {
   ntp_promo->GetInteger(kPrefPromoGroup, &group_);
   ntp_promo->GetInteger(kPrefPromoViews, &views_);
   ntp_promo->GetBoolean(kPrefPromoClosed, &closed_);
-
-  ntp_promo->GetBoolean(kPrefPromoGPlusRequired, &gplus_required_);
 }
 
 bool NotificationPromo::CanShow() const {
@@ -399,29 +397,74 @@ bool NotificationPromo::CanShow() const {
          !ExceedsMaxGroup() &&
          !ExceedsMaxViews() &&
          base::Time::FromDoubleT(StartTimeForGroup()) < base::Time::Now() &&
-         base::Time::FromDoubleT(EndTime()) > base::Time::Now() &&
-         IsGPlusRequired();
+         base::Time::FromDoubleT(EndTime()) > base::Time::Now();
 }
 
 // static
-void NotificationPromo::HandleClosed(Profile* profile, PromoType promo_type) {
+void NotificationPromo::HandleClosed(PromoType promo_type) {
   content::RecordAction(UserMetricsAction("NTPPromoClosed"));
-  NotificationPromo promo(profile);
-  promo.InitFromPrefs(promo_type);
-  if (!promo.closed_) {
-    promo.closed_ = true;
-    promo.WritePrefs();
-  }
+  NotificationPromo promo;
+  HandleClosedInternal(&promo, promo_type);
 }
 
 // static
-bool NotificationPromo::HandleViewed(Profile* profile, PromoType promo_type) {
+bool NotificationPromo::HandleViewed(PromoType promo_type) {
   content::RecordAction(UserMetricsAction("NTPPromoShown"));
-  NotificationPromo promo(profile);
-  promo.InitFromPrefs(promo_type);
-  ++promo.views_;
-  promo.WritePrefs();
-  return promo.ExceedsMaxViews();
+  NotificationPromo promo;
+  return HandleViewedInternal(&promo, promo_type);
+}
+
+// static
+void NotificationPromo::HandleClosedInternal(NotificationPromo* promo,
+                                             PromoType promo_type) {
+  promo->InitFromPrefs(promo_type);
+  if (!promo->closed_) {
+    promo->closed_ = true;
+    promo->WritePrefs();
+  }
+}
+
+// static
+bool NotificationPromo::HandleViewedInternal(NotificationPromo* promo,
+                                             PromoType promo_type) {
+  promo->InitFromPrefs(promo_type);
+  ++promo->views_;
+  promo->WritePrefs();
+  return promo->ExceedsMaxViews();
+}
+
+// Testing constructor.
+NotificationPromo::NotificationPromo(PrefService* prefs)
+    : prefs_(prefs),
+      promo_type_(NO_PROMO),
+      promo_payload_(new base::DictionaryValue()),
+      start_(0.0),
+      end_(0.0),
+      num_groups_(kDefaultGroupSize),
+      initial_segment_(0),
+      increment_(1),
+      time_slice_(0),
+      max_group_(0),
+      max_views_(0),
+      group_(0),
+      views_(0),
+      closed_(false),
+      new_notification_(false) {
+  DCHECK(prefs_);
+}
+
+// static
+void NotificationPromo::HandleClosedForTesting(PrefService* prefs,
+                                               PromoType promo_type) {
+  NotificationPromo testing_promo(prefs);
+  HandleClosedInternal(&testing_promo, promo_type);
+}
+
+// static
+bool NotificationPromo::HandleViewedForTesting(PrefService* prefs,
+                                               PromoType promo_type) {
+  NotificationPromo testing_promo(prefs);
+  return HandleViewedInternal(&testing_promo, promo_type);
 }
 
 bool NotificationPromo::ExceedsMaxGroup() const {
@@ -432,10 +475,6 @@ bool NotificationPromo::ExceedsMaxViews() const {
   return (max_views_ == 0) ? false : views_ >= max_views_;
 }
 
-bool NotificationPromo::IsGPlusRequired() const {
-  return !gplus_required_ || prefs_->GetBoolean(prefs::kIsGooglePlusUser);
-}
-
 // static
 GURL NotificationPromo::PromoServerURL() {
   GURL url(promo_server_url);