promo_resource_service fixes/cleanup for promos.

chrome/browser/web_resource/promo_resource_service.cc

 // Copyright (c) 2011 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/web_resource/promo_resource_service.h"
 
 #include "base/command_line.h"
 #include "base/string_number_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
@@ skipping 16 matching lines @@
 
 // Delay on first fetch so we don't interfere with startup.
 static const int kStartResourceFetchDelay = 5000;
 
 // Delay between calls to update the cache (48 hours).
 static const int kCacheUpdateDelay = 48 * 60 * 60 * 1000;
 
 // Users are randomly assigned to one of kNTPPromoGroupSize buckets, in order
 // to be able to roll out promos slowly, or display different promos to
 // different groups.
-static const int kNTPPromoGroupSize = 16;
+static const int kNTPPromoGroupSize = 100;
 
 // Maximum number of hours for each time slice (4 weeks).
 static const int kMaxTimeSliceHours = 24 * 7 * 4;
 
 // The version of the service (used to expire the cache when upgrading Chrome
 // to versions with different types of promos).
 static const int kPromoServiceVersion = 2;
 
 // Properties used by the server.
 static const char kAnswerIdProperty[] = "answer_id";
 static const char kWebStoreHeaderProperty[] = "question";
 static const char kWebStoreButtonProperty[] = "inproduct_target";
 static const char kWebStoreLinkProperty[] = "inproduct";
 static const char kWebStoreExpireProperty[] = "tooltip";
 
+chrome::VersionInfo::Channel GetChannel() {
+  // GetChannel hits the registry on Windows. See http://crbug.com/70898.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  return chrome::VersionInfo::GetChannel();
+}
+
+int GetNextQuestionValue(const std::string& question,
+                         size_t* index,
+                         bool* err) {
+  if (*err)
+    return 0;
+
+  size_t new_index = question.find(':', *index);
+  // Note that substr correctly handles npos.
+  std::string fragment(question.substr(*index, new_index - *index));
+  *index = new_index + 1;
+
+  int value;
+  *err = !base::StringToInt(fragment, &value);
+  return value;
+}
+
 const char* GetPromoResourceURL() {
   std::string promo_server_url = CommandLine::ForCurrentProcess()->
       GetSwitchValueASCII(switches::kPromoServerURL);
   return promo_server_url.empty() ?
       PromoResourceService::kDefaultPromoResourceServer :
       promo_server_url.c_str();
 }
 
 }  // namespace
 
-// Server for dynamically loaded NTP HTML elements. TODO(mirandac): append
-// locale for future usage, when we're serving localizable strings.
+// Server for dynamically loaded NTP HTML elements.
 const char* PromoResourceService::kDefaultPromoResourceServer =
     "https://www.google.com/support/chrome/bin/topic/1142433/inproduct?hl=";
 
 // static
 void PromoResourceService::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterIntegerPref(prefs::kNTPPromoVersion, 0);
   local_state->RegisterStringPref(prefs::kNTPPromoLocale, std::string());
 }
 
 // static
@@ skipping 10 matching lines @@
   prefs->RegisterDoublePref(prefs::kNTPPromoEnd,
                             0,
                             PrefService::UNSYNCABLE_PREF);
   prefs->RegisterStringPref(prefs::kNTPPromoLine,
                             std::string(),
                             PrefService::UNSYNCABLE_PREF);
   prefs->RegisterBooleanPref(prefs::kNTPPromoClosed,
                              false,
                              PrefService::UNSYNCABLE_PREF);
   prefs->RegisterIntegerPref(prefs::kNTPPromoGroup,
-                             -1,
+                             0,
                              PrefService::UNSYNCABLE_PREF);
   prefs->RegisterIntegerPref(
       prefs::kNTPPromoBuild,
       CANARY_BUILD | DEV_BUILD | BETA_BUILD | STABLE_BUILD,
       PrefService::UNSYNCABLE_PREF);
   prefs->RegisterIntegerPref(prefs::kNTPPromoGroupTimeSlice,
                              0,
                              PrefService::UNSYNCABLE_PREF);
+  prefs->RegisterIntegerPref(prefs::kNTPPromoGroupMax,
+                             0,
+                             PrefService::UNSYNCABLE_PREF);
 }
 
 // static
 bool PromoResourceService::IsBuildTargeted(chrome::VersionInfo::Channel channel,
                                            int builds_allowed) {
   if (builds_allowed == NO_BUILD)
     return false;
   switch (channel) {
     case chrome::VersionInfo::CHANNEL_CANARY:
       return (CANARY_BUILD & builds_allowed) != 0;
@@ skipping 11 matching lines @@
 }
 
 PromoResourceService::PromoResourceService(Profile* profile)
     : WebResourceService(profile->GetPrefs(),
                          GetPromoResourceURL(),
                          true,  // append locale to URL
                          chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED,
                          prefs::kNTPPromoResourceCacheUpdate,
                          kStartResourceFetchDelay,
                          kCacheUpdateDelay),
-      profile_(profile),
-      web_resource_cache_(NULL),
-      channel_(chrome::VersionInfo::CHANNEL_UNKNOWN) {
+                         profile_(profile),
+                         channel_(chrome::VersionInfo::CHANNEL_UNKNOWN) {
   Init();
 }
 
 PromoResourceService::~PromoResourceService() { }
 
 void PromoResourceService::Init() {
   ScheduleNotificationOnInit();
 }
 
 bool PromoResourceService::IsThisBuildTargeted(int builds_targeted) {
-  if (channel_ == chrome::VersionInfo::CHANNEL_UNKNOWN) {
-    // GetChannel hits the registry on Windows. See http://crbug.com/70898.
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    channel_ = chrome::VersionInfo::GetChannel();
-  }
+  if (channel_ == chrome::VersionInfo::CHANNEL_UNKNOWN)
+    channel_ = GetChannel();
 
   return IsBuildTargeted(channel_, builds_targeted);
 }
 
 void PromoResourceService::Unpack(const DictionaryValue& parsed_json) {
   UnpackLogoSignal(parsed_json);
-  UnpackPromoSignal(parsed_json);
+  UnpackAdSignal(parsed_json);
   UnpackWebStoreSignal(parsed_json);
 }
 
 void PromoResourceService::ScheduleNotification(double promo_start,
                                                 double promo_end) {
   if (promo_start > 0 && promo_end > 0) {
     int64 ms_until_start =
         static_cast<int64>((base::Time::FromDoubleT(
             promo_start) - base::Time::Now()).InMilliseconds());
     int64 ms_until_end =
@@ skipping 35 matching lines @@
 int PromoResourceService::GetPromoServiceVersion() {
   PrefService* local_state = g_browser_process->local_state();
   return local_state->GetInteger(prefs::kNTPPromoVersion);
 }
 
 std::string PromoResourceService::GetPromoLocale() {
   PrefService* local_state = g_browser_process->local_state();
   return local_state->GetString(prefs::kNTPPromoLocale);
 }
 
-void PromoResourceService::UnpackPromoSignal(
+void PromoResourceService::UnpackAdSignal(

[Miranda Callahan, 2011/09/15 08:21:30]

nit:UnpackNotificationSignal

[achuithb, 2011/09/15 18:58:13]

Done.

     const DictionaryValue& parsed_json) {
+  // Check for newly received start and end values.
+  std::string promo_start_string, promo_end_string;
+
   DictionaryValue* topic_dict;
-  ListValue* answer_list;
-  double old_promo_start = 0;
-  double old_promo_end = 0;
-  double promo_start = 0;
-  double promo_end = 0;
-
-  // Check for preexisting start and end values.
-  if (prefs_->HasPrefPath(prefs::kNTPPromoStart) &&
-      prefs_->HasPrefPath(prefs::kNTPPromoEnd)) {
-    old_promo_start = prefs_->GetDouble(prefs::kNTPPromoStart);
-    old_promo_end = prefs_->GetDouble(prefs::kNTPPromoEnd);
-  }
-
-  // Check for newly received start and end values.
   if (parsed_json.GetDictionary("topic", &topic_dict)) {
+    ListValue* answer_list;
     if (topic_dict->GetList("answers", &answer_list)) {
-      std::string promo_start_string = "";
-      std::string promo_end_string = "";
-      std::string promo_string = "";
-      std::string promo_build = "";
-      int promo_build_type = 0;
-      int time_slice_hrs = 0;
       for (ListValue::const_iterator answer_iter = answer_list->begin();
            answer_iter != answer_list->end(); ++answer_iter) {
         if (!(*answer_iter)->IsType(Value::TYPE_DICTIONARY))
           continue;
-        DictionaryValue* a_dic =
-            static_cast<DictionaryValue*>(*answer_iter);
-        std::string promo_signal;
-        if (a_dic->GetString("name", &promo_signal)) {
-          if (promo_signal == "promo_start") {
-            a_dic->GetString("question", &promo_build);
-            size_t split = promo_build.find(":");
-            if (split != std::string::npos &&
-                base::StringToInt(promo_build.substr(0, split),
-                                  &promo_build_type) &&
-                base::StringToInt(promo_build.substr(split+1),
-                                  &time_slice_hrs) &&
-                promo_build_type >= 0 &&
-                promo_build_type <= (DEV_BUILD | BETA_BUILD | STABLE_BUILD) &&
-                time_slice_hrs >= 0 &&
-                time_slice_hrs <= kMaxTimeSliceHours) {
-              prefs_->SetInteger(prefs::kNTPPromoBuild, promo_build_type);
-              prefs_->SetInteger(prefs::kNTPPromoGroupTimeSlice,
-                                 time_slice_hrs);
-            } else {
-              // If no time data or bad time data are set, do not show promo.
-              prefs_->SetInteger(prefs::kNTPPromoBuild, NO_BUILD);
-              prefs_->SetInteger(prefs::kNTPPromoGroupTimeSlice, 0);
-            }
-            a_dic->GetString("inproduct", &promo_start_string);
-            a_dic->GetString("tooltip", &promo_string);
-            prefs_->SetString(prefs::kNTPPromoLine, promo_string);
-            srand(static_cast<uint32>(time(NULL)));
-            prefs_->SetInteger(prefs::kNTPPromoGroup,
-                               rand() % kNTPPromoGroupSize);
-          } else if (promo_signal == "promo_end") {
-            a_dic->GetString("inproduct", &promo_end_string);
-          }
-        }
-      }
-      if (!promo_start_string.empty() &&
-          promo_start_string.length() > 0 &&
-          !promo_end_string.empty() &&
-          promo_end_string.length() > 0) {
-        base::Time start_time;
-        base::Time end_time;
-        if (base::Time::FromString(promo_start_string.c_str(), &start_time) &&
-            base::Time::FromString(promo_end_string.c_str(), &end_time)) {
-          // Add group time slice, adjusted from hours to seconds.
-          promo_start = start_time.ToDoubleT() +
-              (prefs_->FindPreference(prefs::kNTPPromoGroup) ?
-                  prefs_->GetInteger(prefs::kNTPPromoGroup) *
-                      time_slice_hrs * 60 * 60 : 0);
-          promo_end = end_time.ToDoubleT();
-        }
+
+        ParseNotification(static_cast<DictionaryValue*>(*answer_iter),
+            &promo_start_string, &promo_end_string);
       }
     }
   }
 
+  CheckForNewNotification(promo_start_string, promo_end_string);
+}
+
+void PromoResourceService::ParseNotification(DictionaryValue* a_dic,

[jstritar, 2011/09/15 15:42:27]

nit: DictionaryValue* a_dic should be on the next line, indented with the
others. or they could all line up under DictionaryValue*

[achuithb, 2011/09/15 18:58:13]

Done.

+    std::string* promo_start_string,
+    std::string* promo_end_string) {
+  std::string promo_signal;
+  if (a_dic->GetString("name", &promo_signal)) {
+    if (promo_signal == "promo_start") {
+      SetNotificationParams(a_dic);
+      SetNotificationLine(a_dic);
+
+      a_dic->GetString("inproduct", promo_start_string);
+    } else if (promo_signal == "promo_end") {
+      a_dic->GetString("inproduct", promo_end_string);
+    }
+  }
+}
+
+void PromoResourceService::SetNotificationParams(DictionaryValue* a_dic) {
+  std::string question;
+  a_dic->GetString("question", &question);
+
+  size_t index(0);
+  bool err(false);
+  int promo_build = GetNextQuestionValue(question, &index, &err);
+  int time_slice = GetNextQuestionValue(question, &index, &err);
+  int max_group = GetNextQuestionValue(question, &index, &err);
+
+  if (err ||
+      promo_build < 0 ||
+      promo_build > (DEV_BUILD | BETA_BUILD | STABLE_BUILD) ||

[jstritar, 2011/09/15 15:42:27]

There's also a CANARY_BUILD now. How about we define a MAX_BUILD or ALL_BUILDS
equal to (1 << 4) - 1 that we can use here so we don't forget to update this in
the future?

[achuithb, 2011/09/15 18:58:13]

Done.

+      time_slice < 0 ||
+      time_slice > kMaxTimeSliceHours ||
+      max_group < 0 ||
+      max_group >= kNTPPromoGroupSize) {
+    // If values are not valid, do not show promo.
+    NOTREACHED() << "Invalid server data, question=" << question <<
+        ", build=" << promo_build <<
+        ", time_slice=" << time_slice <<
+        ", max_group=" << max_group;
+    promo_build = NO_BUILD;
+    time_slice = 0;
+    max_group = 0;
+  }
+
+  prefs_->SetInteger(prefs::kNTPPromoBuild, promo_build);
+  prefs_->SetInteger(prefs::kNTPPromoGroupTimeSlice, time_slice);
+  prefs_->SetInteger(prefs::kNTPPromoGroupMax, max_group);
+}
+
+void PromoResourceService::SetNotificationLine(DictionaryValue* a_dic) {
+  std::string promo_line;
+  a_dic->GetString("tooltip", &promo_line);
+  if (!promo_line.empty())
+    prefs_->SetString(prefs::kNTPPromoLine, promo_line);
+}
+
+void PromoResourceService::CheckForNewNotification(
+    const std::string& promo_start_string,
+    const std::string& promo_end_string) {
+  double promo_start = 0.0, promo_end = 0.0;
+  CalculateNewNotificationTimes(promo_start_string, promo_end_string,
+      &promo_start, &promo_end);
+
+  double old_promo_start = 0.0, old_promo_end = 0.0;
+  GetCurrentNotificationTimes(&old_promo_start, &old_promo_end);
+
   // If start or end times have changed, trigger a new web resource
   // notification, so that the logo on the NTP is updated. This check is
   // outside the reading of the web resource data, because the absence of
   // dates counts as a triggering change if there were dates before.
-  // Also reset the promo closed preference, to signal a new promo.
-  if (!(old_promo_start == promo_start) ||
-      !(old_promo_end == promo_end)) {
-    prefs_->SetDouble(prefs::kNTPPromoStart, promo_start);
-    prefs_->SetDouble(prefs::kNTPPromoEnd, promo_end);
-    prefs_->SetBoolean(prefs::kNTPPromoClosed, false);
-    ScheduleNotification(promo_start, promo_end);
-  }
+  // Also create new promo groups, and reset the promo closed preference,
+  // to signal a new promo.
+  if (old_promo_start != promo_start || old_promo_end != promo_end) {
+    OnNewNotification(promo_start, promo_end);

[jstritar, 2011/09/15 15:42:27]

nit: don't need the { }

[achuithb, 2011/09/15 18:58:13]

Done.

+  }
+}
+void PromoResourceService::CalculateNewNotificationTimes(

[jstritar, 2011/09/15 15:42:27]

Would ParseNewNotificationTimes be a better name, since it's not doing many
calculations (like adding group offsets)?

[achuithb, 2011/09/15 18:58:13]

Done.

+    const std::string& promo_start_string,
+    const std::string& promo_end_string,
+    double* promo_start,
+    double* promo_end) {
+  *promo_start = *promo_end = 0.0;
+
+  if (promo_start_string.empty() && !promo_end_string.empty())
+    return;
+
+  base::Time start_time, end_time;
+  if (!base::Time::FromString(promo_start_string.c_str(), &start_time) ||
+      !base::Time::FromString(promo_end_string.c_str(), &end_time))
+    return;
+
+  *promo_start = start_time.ToDoubleT();
+  *promo_end = end_time.ToDoubleT();
+}
+
+void PromoResourceService::GetCurrentNotificationTimes(double* old_promo_start,
+                                                 double* old_promo_end) {

[jstritar, 2011/09/15 15:42:27]

nit: indent

[achuithb, 2011/09/15 18:58:13]

Done.

+  *old_promo_start = *old_promo_end = 0.0;
+  if (prefs_->HasPrefPath(prefs::kNTPPromoStart) &&
+      prefs_->HasPrefPath(prefs::kNTPPromoEnd)) {
+    *old_promo_start = prefs_->GetDouble(prefs::kNTPPromoStart);
+    *old_promo_end = prefs_->GetDouble(prefs::kNTPPromoEnd);
+  }
+}
+
+int PromoResourceService::ResetNotificationGroup() {
+  srand(static_cast<uint32>(time(NULL)));
+  const int promo_group = rand() % kNTPPromoGroupSize;
+  prefs_->SetInteger(prefs::kNTPPromoGroup, promo_group);
+  return promo_group;
+}
+
+// static
+double PromoResourceService::GetNotificationStartTime(PrefService* prefs) {
+  if (!prefs->HasPrefPath(prefs::kNTPPromoStart))
+    return 0.0;
+
+  const double promo_start = prefs->GetDouble(prefs::kNTPPromoStart);
+
+  if (!prefs->HasPrefPath(prefs::kNTPPromoGroup) ||
+      !prefs->HasPrefPath(prefs::kNTPPromoGroupTimeSlice))
+    return promo_start;
+
+  const int promo_group = prefs->GetInteger(prefs::kNTPPromoGroup);
+  const int time_slice = prefs->GetInteger(prefs::kNTPPromoGroupTimeSlice);
+  // Adjust promo_start using group time slice, adjusted from hours to seconds.
+  return promo_start + promo_group * time_slice * 60 * 60;

[jstritar, 2011/09/15 15:42:27]

nit: maybe put 60*60 in a constant?

[achuithb, 2011/09/15 18:58:13]

Done.

+}
+
+void PromoResourceService::OnNewNotification(double promo_start,
+                                             double promo_end) {
+  ResetNotificationGroup();
+
+  prefs_->SetBoolean(prefs::kNTPPromoClosed, false);
+
+  prefs_->SetDouble(prefs::kNTPPromoStart, promo_start);
+  prefs_->SetDouble(prefs::kNTPPromoEnd, promo_end);
+
+  ScheduleNotification(GetNotificationStartTime(prefs_), promo_end);
+}
+
+bool PromoResourceService::CanShowPromo(Profile* profile) {
+  PrefService* prefs = profile->GetPrefs();
+
+  // Check if promo has been closed by the user.
+  if (prefs->HasPrefPath(prefs::kNTPPromoClosed) &&
+      prefs->GetBoolean(prefs::kNTPPromoClosed))
+    return false;
+
+  // Check if our build is appropriate for this promo.
+  if (!prefs->HasPrefPath(prefs::kNTPPromoBuild) ||
+      !IsBuildTargeted(GetChannel(), prefs->GetInteger(prefs::kNTPPromoBuild)))
+    return false;
+
+  // Check if we are in the right group for this promo.
+  if (!prefs->FindPreference(prefs::kNTPPromoGroup) ||
+      !prefs->FindPreference(prefs::kNTPPromoGroupMax) ||
+      (prefs->GetInteger(prefs::kNTPPromoGroup) >=
+      prefs->GetInteger(prefs::kNTPPromoGroupMax)))
+    return false;
+
+  // Check if we are in the right time window for this promo.
+  if (!prefs->FindPreference(prefs::kNTPPromoStart) ||
+      !prefs->FindPreference(prefs::kNTPPromoEnd) ||
+      base::Time::FromDoubleT(GetNotificationStartTime(prefs)) >
+          base::Time::Now() ||
+      base::Time::FromDoubleT(prefs->GetDouble(prefs::kNTPPromoEnd)) <
+          base::Time::Now())
+    return false;
+
+  return prefs->HasPrefPath(prefs::kNTPPromoLine);
 }
 
 void PromoResourceService::UnpackWebStoreSignal(
     const DictionaryValue& parsed_json) {
   DictionaryValue* topic_dict;
   ListValue* answer_list;
 
   bool is_webstore_active = false;
   bool signal_found = false;
   AppsPromo::PromoData promo_data;
@@ skipping 131 matching lines @@
   if (!(old_logo_start == logo_start) ||
       !(old_logo_end == logo_end)) {
     prefs_->SetDouble(prefs::kNTPCustomLogoStart, logo_start);
     prefs_->SetDouble(prefs::kNTPCustomLogoEnd, logo_end);
     NotificationService* service = NotificationService::current();
     service->Notify(chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED,
                     Source<WebResourceService>(this),
                     NotificationService::NoDetails());
   }
 }
-
-namespace PromoResourceServiceUtil {
-
-bool CanShowPromo(Profile* profile) {
-  bool promo_closed = false;
-  PrefService* prefs = profile->GetPrefs();
-  if (prefs->HasPrefPath(prefs::kNTPPromoClosed))
-    promo_closed = prefs->GetBoolean(prefs::kNTPPromoClosed);
-
-  // Only show if not synced.
-  bool is_synced =
-      (profile->HasProfileSyncService() &&
-          sync_ui_util::GetStatus(
-              profile->GetProfileSyncService()) == sync_ui_util::SYNCED);
-
-  bool is_promo_build = false;
-  if (prefs->HasPrefPath(prefs::kNTPPromoBuild)) {
-    // GetChannel hits the registry on Windows. See http://crbug.com/70898.
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
-    is_promo_build = PromoResourceService::IsBuildTargeted(
-        channel, prefs->GetInteger(prefs::kNTPPromoBuild));
-  }
-
-  return !promo_closed && !is_synced && is_promo_build;
-}
-
-}  // namespace PromoResourceServiceUtil