Support for promo_type.

* Create an enum PromoType and a data member promo_type_ of NotificationPromo.
* InitFromJson, InitFromPrefs, HandleClosed and HandleViewed all take a PromoType now.
* HandleClosed and HandleViewed are static.
* Add an anon function PromoTypeToString for conversion from PromoType to string.
* Improve unit tests TestViewed() and TestClosed().

BUG=123061
TEST=unit tests. Manual.

Review URL: https://chromiumcodereview.appspot.com/10836099

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150235 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/webui/ntp/new_tab_page_handler.cc

@@ -63,15 +63,16 @@ void NewTabPageHandler::RegisterMessages() {
 }
 
 void NewTabPageHandler::HandleCloseNotificationPromo(const ListValue* args) {
-  NotificationPromo notification_promo(Profile::FromWebUI(web_ui()));
-  notification_promo.HandleClosed();
+  NotificationPromo::HandleClosed(Profile::FromWebUI(web_ui()),
+                                  NotificationPromo::NTP_NOTIFICATION_PROMO);
   Notify(chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED);
 }
 
 void NewTabPageHandler::HandleNotificationPromoViewed(const ListValue* args) {
-  NotificationPromo notification_promo(Profile::FromWebUI(web_ui()));
-  if (notification_promo.HandleViewed())
+  if (NotificationPromo::HandleViewed(Profile::FromWebUI(web_ui()),
+        NotificationPromo::NTP_NOTIFICATION_PROMO)) {
     Notify(chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED);
+  }
 }
 
 void NewTabPageHandler::HandlePageSelected(const ListValue* args) {

chrome/browser/ui/webui/ntp/ntp_resource_cache.cc

@@ -410,7 +410,7 @@ void NTPResourceCache::CreateNewTabHTML() {
 
   // Set the promo string for display if there is a valid outstanding promo.
   NotificationPromo notification_promo(profile_);
-  notification_promo.InitFromPrefs();
+  notification_promo.InitFromPrefs(NotificationPromo::NTP_NOTIFICATION_PROMO);
   if (notification_promo.CanShow())
     load_time_data.SetString("serverpromo", notification_promo.promo_text());
 

chrome/browser/web_resource/notification_promo.cc

@@ -105,6 +105,28 @@ const char* ChannelString() {
   }
 }
 
+struct PromoMapEntry {
+  NotificationPromo::PromoType promo_type;
+  const char* promo_type_str;
+};
+
+const PromoMapEntry kPromoMap[] = {
+    { NotificationPromo::NO_PROMO, "" },
+    { NotificationPromo::NTP_NOTIFICATION_PROMO, "ntp_notification_promo" },
+    { NotificationPromo::BUBBLE_PROMO, "bubble_promo" },
+    { NotificationPromo::MOBILE_NTP_SYNC_PROMO, "mobile_ntp_sync_promo" },
+};
+
+// Convert PromoType to appropriate string.
+const char* PromoTypeToString(NotificationPromo::PromoType promo_type) {
+  for (size_t i = 0; i < arraysize(kPromoMap); ++i) {
+    if (kPromoMap[i].promo_type == promo_type)
+      return kPromoMap[i].promo_type_str;
+  }
+  NOTREACHED();
+  return "";
+}
+
 // TODO(achuith): remove this in m23.
 void ClearDeprecatedPrefs(PrefService* prefs) {
   prefs->RegisterStringPref(prefs::kNtpPromoLine,
@@ -183,17 +205,13 @@ void ClearDeprecatedPrefs(PrefService* prefs) {
 
 }  // namespace
 
-const char NotificationPromo::kNtpNotificationPromoType[] =
-    "ntp_notification_promo";
-const char NotificationPromo::kBubblePromoType[] = "bubble_promo";
-
 NotificationPromo::NotificationPromo(Profile* profile)
     : profile_(profile),
       prefs_(profile_->GetPrefs()),
-      promo_type_(kNtpNotificationPromoType),
+      promo_type_(NO_PROMO),
 #if defined(OS_ANDROID)
       promo_action_args_(new base::ListValue),
-#endif  // defined(OS_ANDROID)
+#endif
       start_(0.0),
       end_(0.0),
       num_groups_(kDefaultGroupSize),
@@ -213,17 +231,14 @@ NotificationPromo::NotificationPromo(Profile* profile)
 
 NotificationPromo::~NotificationPromo() {}
 
-void NotificationPromo::InitFromJson(const DictionaryValue& json) {
+void NotificationPromo::InitFromJson(const DictionaryValue& json,
+                                     PromoType promo_type) {
+  promo_type_ = promo_type;
   const ListValue* promo_list = NULL;
-#if !defined(OS_ANDROID)
-  if (!json.GetList(promo_type_, &promo_list))
-    return;
-#else
-  if (!json.GetList("mobile_ntp_sync_promo", &promo_list)) {
-    LOG(ERROR) << "Malfromed JSON: not a mobile_ntp_sync_promo";
+  if (!json.GetList(PromoTypeToString(promo_type_), &promo_list)) {
+    LOG(ERROR) << "Malformed JSON: not " << PromoTypeToString(promo_type_);
     return;
   }
-#endif  // !defined(OS_ANDROID)
 
   // No support for multiple promos yet. Only consider the first one.
   const DictionaryValue* promo = NULL;
@@ -343,7 +358,7 @@ void NotificationPromo::InitFromJson(const DictionaryValue& json) {
 
 void NotificationPromo::CheckForNewNotification() {
   NotificationPromo old_promo(profile_);
-  old_promo.InitFromPrefs();
+  old_promo.InitFromPrefs(promo_type_);
   const double old_start = old_promo.start_;
   const double old_end = old_promo.end_;
   const std::string old_promo_text = old_promo.promo_text_;
@@ -400,18 +415,19 @@ void NotificationPromo::WritePrefs() {
   promo_list->Set(0, ntp_promo);  // Only support 1 promo for now.
 
   base::DictionaryValue promo_dict;
-  promo_dict.Set(promo_type_, promo_list);
+  promo_dict.Set(PromoTypeToString(promo_type_), promo_list);
   prefs_->Set(kPrefPromoObject, promo_dict);
 }
 
-void NotificationPromo::InitFromPrefs() {
+void NotificationPromo::InitFromPrefs(PromoType promo_type) {
+  promo_type_ = promo_type;
   const base::DictionaryValue* promo_dict =
       prefs_->GetDictionary(kPrefPromoObject);
   if (!promo_dict)
     return;
 
   const base::ListValue* promo_list(NULL);
-  promo_dict->GetList(promo_type_, &promo_list);
+  promo_dict->GetList(PromoTypeToString(promo_type_), &promo_list);
   if (!promo_list)
     return;
 
@@ -458,21 +474,25 @@ bool NotificationPromo::CanShow() const {
          IsGPlusRequired();
 }
 
-void NotificationPromo::HandleClosed() {
+// static
+void NotificationPromo::HandleClosed(Profile* profile, PromoType promo_type) {
   content::RecordAction(UserMetricsAction("NTPPromoClosed"));
-  InitFromPrefs();
-  if (!closed_) {
-    closed_ = true;
-    WritePrefs();
+  NotificationPromo promo(profile);
+  promo.InitFromPrefs(promo_type);
+  if (!promo.closed_) {
+    promo.closed_ = true;
+    promo.WritePrefs();
   }
 }
 
-bool NotificationPromo::HandleViewed() {
+// static
+bool NotificationPromo::HandleViewed(Profile* profile, PromoType promo_type) {
   content::RecordAction(UserMetricsAction("NTPPromoShown"));
-  InitFromPrefs();
-  ++views_;
-  WritePrefs();
-  return ExceedsMaxViews();
+  NotificationPromo promo(profile);
+  promo.InitFromPrefs(promo_type);
+  ++promo.views_;
+  promo.WritePrefs();
+  return promo.ExceedsMaxViews();
 }
 
 bool NotificationPromo::ExceedsMaxGroup() const {

chrome/browser/web_resource/notification_promo.h

@@ -27,15 +27,19 @@ class NotificationPromo {
  public:
   static GURL PromoServerURL();
 
-  static const char kNtpNotificationPromoType[];
-  static const char kBubblePromoType[];
+  enum PromoType {
+    NO_PROMO,
+    NTP_NOTIFICATION_PROMO,
+    BUBBLE_PROMO,
+    MOBILE_NTP_SYNC_PROMO,
+  };
 
   explicit NotificationPromo(Profile* profile);
   ~NotificationPromo();
 
   // Initialize from json/prefs.
-  void InitFromJson(const base::DictionaryValue& json);
-  void InitFromPrefs();
+  void InitFromJson(const base::DictionaryValue& json, PromoType promo_type);
+  void InitFromPrefs(PromoType promo_type);
 
   // Can this promo be shown?
   bool CanShow() const;
@@ -46,8 +50,11 @@ class NotificationPromo {
   double EndTime() const;
 
   // Helpers for NewTabPageHandler.
-  void HandleClosed();
-  bool HandleViewed();  // returns true if views exceeds maximum allowed.
+  // Mark the promo as closed when the user dismisses it.
+  static void HandleClosed(Profile* profile, PromoType promo_type);
+  // Mark the promo has having been viewed. This returns true if views
+  // exceeds the maximum allowed.
+  static bool HandleViewed(Profile* profile, PromoType promo_type);
 
   bool new_notification() const { return new_notification_; }
 
@@ -91,7 +98,7 @@ class NotificationPromo {
   Profile* profile_;
   PrefService* prefs_;
 
-  std::string promo_type_;
+  PromoType promo_type_;
   std::string promo_text_;
 #if defined(OS_ANDROID) || defined(OS_IOS)
   std::string promo_text_long_;

chrome/browser/web_resource/promo_resource_service.cc

@@ -179,7 +179,13 @@ std::string PromoResourceService::GetPromoLocale() {
 
 void PromoResourceService::Unpack(const DictionaryValue& parsed_json) {
   NotificationPromo notification_promo(profile_);
-  notification_promo.InitFromJson(parsed_json);
+  NotificationPromo::PromoType promo_type =
+#if !defined(OS_ANDROID)
+    NotificationPromo::NTP_NOTIFICATION_PROMO;
+#else
+    NotificationPromo::MOBILE_NTP_SYNC_PROMO;
+#endif
+  notification_promo.InitFromJson(parsed_json, promo_type);
 
   if (notification_promo.new_notification()) {
     ScheduleNotification(notification_promo.StartTimeForGroup(),

chrome/browser/web_resource/promo_resource_service_unittest.cc

@@ -77,6 +77,13 @@ class NotificationPromoTest {
     ASSERT_TRUE(dict);
     test_json_.reset(dict);
 
+    promo_type_ =
+#if !defined(OS_ANDROID)
+        NotificationPromo::NTP_NOTIFICATION_PROMO;
+#else
+        NotificationPromo::MOBILE_NTP_SYNC_PROMO;
+#endif
+
     promo_text_ = promo_text;
 
 #if defined(OS_ANDROID)
@@ -104,7 +111,7 @@ class NotificationPromoTest {
   }
 
   void InitPromoFromJson(bool should_receive_notification) {
-    notification_promo_.InitFromJson(*test_json_);
+    notification_promo_.InitFromJson(*test_json_, promo_type_);
     EXPECT_EQ(should_receive_notification,
               notification_promo_.new_notification());
 
@@ -156,7 +163,7 @@ class NotificationPromoTest {
   // notification.
   void TestInitFromPrefs() {
     NotificationPromo prefs_notification_promo(profile_);
-    prefs_notification_promo.InitFromPrefs();
+    prefs_notification_promo.InitFromPrefs(promo_type_);
 
     EXPECT_EQ(notification_promo_.prefs_,
               prefs_notification_promo.prefs_);
@@ -236,44 +243,44 @@ class NotificationPromoTest {
     notification_promo_.views_ = notification_promo_.max_views_ - 2;
     notification_promo_.WritePrefs();
 
+    NotificationPromo::HandleViewed(profile_, promo_type_);
     NotificationPromo new_promo(profile_);
-    new_promo.HandleViewed();
+    new_promo.InitFromPrefs(promo_type_);
+    EXPECT_EQ(new_promo.max_views_ - 1, new_promo.views_);
     EXPECT_TRUE(new_promo.CanShow());
-    new_promo.HandleViewed();
+    NotificationPromo::HandleViewed(profile_, promo_type_);
+    new_promo.InitFromPrefs(promo_type_);
+    EXPECT_EQ(new_promo.max_views_, new_promo.views_);
     EXPECT_FALSE(new_promo.CanShow());
 
-    notification_promo_.InitFromPrefs();
-    EXPECT_FALSE(notification_promo_.CanShow());
-
     // Test out of range views.
     for (int i = max_views_; i < max_views_ * 2; ++i) {
-      notification_promo_.views_ = i;
-      EXPECT_FALSE(notification_promo_.CanShow());
+      new_promo.views_ = i;
+      EXPECT_FALSE(new_promo.CanShow());
     }
 
     // Test in range views.
     for (int i = 0; i < max_views_; ++i) {
-      notification_promo_.views_ = i;
-      EXPECT_TRUE(notification_promo_.CanShow());
+      new_promo.views_ = i;
+      EXPECT_TRUE(new_promo.CanShow());
     }
-    notification_promo_.WritePrefs();
+    new_promo.WritePrefs();
   }
 
   void TestClosed() {
     NotificationPromo new_promo(profile_);
-    new_promo.InitFromPrefs();
+    new_promo.InitFromPrefs(promo_type_);
+    EXPECT_FALSE(new_promo.closed_);
     EXPECT_TRUE(new_promo.CanShow());
-    new_promo.HandleClosed();
-    EXPECT_FALSE(new_promo.CanShow());
-    new_promo.InitFromPrefs();
-    EXPECT_FALSE(new_promo.CanShow());
 
-    notification_promo_.closed_ = true;
-    EXPECT_FALSE(notification_promo_.CanShow());
+    NotificationPromo::HandleClosed(profile_, promo_type_);
+    new_promo.InitFromPrefs(promo_type_);
+    EXPECT_TRUE(new_promo.closed_);
+    EXPECT_FALSE(new_promo.CanShow());
 
-    notification_promo_.closed_ = false;
-    EXPECT_TRUE(notification_promo_.CanShow());
-    notification_promo_.WritePrefs();
+    new_promo.closed_ = false;
+    EXPECT_TRUE(new_promo.CanShow());
+    new_promo.WritePrefs();
   }
 
   void TestPromoText() {
@@ -378,6 +385,7 @@ class NotificationPromoTest {
   bool received_notification_;
   scoped_ptr<DictionaryValue> test_json_;
 
+  NotificationPromo::PromoType promo_type_;
   std::string promo_text_;
 #if defined(OS_ANDROID)
   std::string promo_text_long_;