Purge legacy notification promo code.

chrome/browser/web_resource/promo_resource_service.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/web_resource/promo_resource_service.h"
 
 #include "base/command_line.h"
 #include "base/rand_util.h"
 #include "base/string_number_conversions.h"
 #include "base/threading/thread_restrictions.h"
@@ skipping 13 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "googleurl/src/gurl.h"
 
 namespace {
 
 // Delay on first fetch so we don't interfere with startup.
 static const int kStartResourceFetchDelay = 5000;
 
 // Delay between calls to update the cache (12 hours), and 3 min in debug mode.
 static const int kCacheUpdateDelay = 12 * 60 * 60 * 1000;
 static const int kTestCacheUpdateDelay = 3 * 60 * 1000;

[Dan Beam, 2012/06/04 18:41:04]

I think this should be a separate flag, personally, so we can change it to less
without modifying the source (and essentially turn it off for tests by making it
really large or something...) or at least add some type of --enable-promos
(which would make tin foils hat wearers and unit tests happy!).

[achuithb, 2012/06/04 19:52:59]

We probably already have too many chrome flags. We should just pick a good debug
value. If 3 min is too long, maybe we should reduce this?

As for unit tests, I think we had a discussion about this (can't remember if it
was me and Rob or me and you?), and concluded that we don't want to do anything
special for tests. In other similar situations, I think we continue to hit
production and the load from tests ends up being is relatively small. We also
decided that we didn't want more code that behaves differently in a test
situation.

 
 // The version of the service (used to expire the cache when upgrading Chrome
 // to versions with different types of promos).
 static const int kPromoServiceVersion = 7;
 
 // 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";
 
 GURL GetPromoResourceURL(bool legacy) {
   const std::string promo_server_url = CommandLine::ForCurrentProcess()->
       GetSwitchValueASCII(switches::kPromoServerURL);
   if (!promo_server_url.empty())
     return GURL(promo_server_url);
   return legacy ? GURL(PromoResourceService::kDefaultPromoResourceServer) :
                   NotificationPromo::PromoServerURL();
 }
 
 bool IsTest() {
   return CommandLine::ForCurrentProcess()->HasSwitch(switches::kPromoServerURL);

[Dan Beam, 2012/06/04 18:20:08]

is it time to add something else here that delays possibly pinging the promo
server during every automated test?

[achuithb, 2012/06/04 19:52:59]

Let's punt this for now.

 }
 
 int GetCacheUpdateDelay() {
   return IsTest() ? kTestCacheUpdateDelay : kCacheUpdateDelay;

[Dan Beam, 2012/06/04 18:41:04]

I'd prefer to separate this logic / control this with more granularity

[achuithb, 2012/06/04 19:52:59]

Punt for now.

 }
 
 }  // namespace
 
 // Server for dynamically loaded NTP HTML elements.
 const char* PromoResourceService::kDefaultPromoResourceServer =

[Dan Beam, 2012/06/04 18:41:04]

legacy

[achuithb, 2012/06/04 19:52:59]

Done.

     "https://www.google.com/support/chrome/bin/topic/1142433/inproduct?hl=";
 
 
 
 // static
 void PromoResourceService::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterIntegerPref(prefs::kNtpPromoVersion, 0);

[Dan Beam, 2012/06/04 18:41:04]

is this still truly global, or can there be different versions per profile?

[achuithb, 2012/06/04 19:52:59]

It makes sense for it to be truly global, but I don't think this code quite
works since we only clear the current profile's prefs. 

I'll add a TODO.

FWIW, I don't think we've changed the promo version for a while.

   local_state->RegisterStringPref(prefs::kNtpPromoLocale, std::string());
 }
 
 // static
 void PromoResourceService::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterStringPref(prefs::kNtpPromoResourceCacheUpdate,
                             "0",
                             PrefService::UNSYNCABLE_PREF);
   prefs->RegisterDoublePref(prefs::kNtpCustomLogoStart,
                             0,
                             PrefService::UNSYNCABLE_PREF);
   prefs->RegisterDoublePref(prefs::kNtpCustomLogoEnd,
                             0,
                             PrefService::UNSYNCABLE_PREF);
   NotificationPromo::RegisterUserPrefs(prefs);
 }
 
 // static
 chrome::VersionInfo::Channel PromoResourceService::GetChannel() {
   // GetChannel hits the registry on Windows. See http://crbug.com/70898.
   base::ThreadRestrictions::ScopedAllowIO allow_io;

[Dan Beam, 2012/06/04 18:41:04]

would it be worth doing an #if defined(OS_WINDOWS) here then (or preferably
lower in the code in GetChannel() perhaps?)

[achuithb, 2012/06/04 19:52:59]

Hopefully we can delete this.

   return chrome::VersionInfo::GetChannel();
 }
 
 // static
 bool PromoResourceService::IsBuildTargeted(chrome::VersionInfo::Channel channel,

[Dan Beam, 2012/06/04 18:41:04]

why do we still need this? or perhaps only keep the flag check?

[achuithb, 2012/06/04 19:52:59]

flag check?

It's used in UnpackWebstoreSignal

[Dan Beam, 2012/06/04 21:26:32]

  return CommandLine::ForCurrentProcess()->HasSwitch(
      switches::kPromoServerURL);
I meant do we use ^ that? It seems that you think we still need it.

                                            int builds_allowed) {
   if (builds_allowed == NO_BUILD ||
       builds_allowed < 0 ||
       builds_allowed > ALL_BUILDS) {
     return false;
   }
   switch (channel) {
     case chrome::VersionInfo::CHANNEL_CANARY:
       return (CANARY_BUILD & builds_allowed) != 0;
     case chrome::VersionInfo::CHANNEL_DEV:
@@ skipping 31 matching lines @@
 
   return IsBuildTargeted(channel_, builds_targeted);
 }
 
 void PromoResourceService::Unpack(const DictionaryValue& parsed_json) {
   UnpackLogoSignal(parsed_json);
   UnpackNotificationSignal(parsed_json);
   UnpackWebStoreSignal(parsed_json);
 }
 
-void PromoResourceService::OnNotificationParsed(double start, double end,
-                                                bool new_notification) {
-  if (new_notification) {
-    ScheduleNotification(start, end);
-  }
-}
-
 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 =
         static_cast<int64>((base::Time::FromDoubleT(
             promo_end) - base::Time::Now()).InMilliseconds());
     if (ms_until_start > 0)
       PostNotification(ms_until_start);
     if (ms_until_end > 0) {
       PostNotification(ms_until_end);
       if (ms_until_start <= 0) {
         // Notify immediately if time is between start and end.
         PostNotification(0);
       }
     }
   }
 }
 
 void PromoResourceService::ScheduleNotificationOnInit() {
   std::string locale = g_browser_process->GetApplicationLocale();
   if ((GetPromoServiceVersion() != kPromoServiceVersion) ||

[Dan Beam, 2012/06/04 18:41:04]

less ()

[achuithb, 2012/06/04 19:52:59]

Done.

       (GetPromoLocale() != locale)) {
     // If the promo service has been upgraded or Chrome switched locales,
     // refresh the promos.
     PrefService* local_state = g_browser_process->local_state();
     local_state->SetInteger(prefs::kNtpPromoVersion, kPromoServiceVersion);
     local_state->SetString(prefs::kNtpPromoLocale, locale);
     prefs_->ClearPref(prefs::kNtpPromoResourceCacheUpdate);
     AppsPromo::ClearPromo();
     PostNotification(0);
   } else {
     // If the promo start is in the future, set a notification task to
     // invalidate the NTP cache at the time of the promo start.
     double promo_start = prefs_->GetDouble(prefs::kNtpPromoStart);
     double promo_end = prefs_->GetDouble(prefs::kNtpPromoEnd);
     ScheduleNotification(promo_start, promo_end);
   }
 }
 
 void PromoResourceService::PostNotification(int64 delay_ms) {
   if (web_resource_update_scheduled_)
     return;
   if (delay_ms > 0) {
     web_resource_update_scheduled_ = true;
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&PromoResourceService::PromoResourceStateChange,
                    weak_ptr_factory_.GetWeakPtr()),
         base::TimeDelta::FromMilliseconds(delay_ms));
   } else if (delay_ms == 0) {

[Dan Beam, 2012/06/04 18:41:04]

I think we should still post a task with 0 ms isntead of doing it in a different
way here

[achuithb, 2012/06/04 19:52:59]

I tried that, and it crashes on Init, possibly because something is not set up
yet? I'd rather not investigate this now, so leaving it as is.

[Dan Beam, 2012/06/04 21:26:32]

TODO() or comment, then?

[achuithb, 2012/06/04 23:06:08]

Done.

     PromoResourceStateChange();
   }
 }
 
 void PromoResourceService::PromoResourceStateChange() {
   web_resource_update_scheduled_ = false;
   content::NotificationService* service =
       content::NotificationService::current();
   service->Notify(chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED,
                   content::Source<WebResourceService>(this),
                   content::NotificationService::NoDetails());
 }
 
 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::UnpackNotificationSignal(
     const DictionaryValue& parsed_json) {
-  scoped_refptr<NotificationPromo> notification_promo =
-      NotificationPromo::Create(profile_, this);
-  notification_promo->InitFromJson(parsed_json);
+  NotificationPromo notification_promo(profile_);
+  notification_promo.InitFromJson(parsed_json);
+
+  if (notification_promo.new_notification()) {
+    ScheduleNotification(notification_promo.StartTimeForGroup(),
+                         notification_promo.EndTime());
+  }
 }
 
 bool PromoResourceService::CanShowNotificationPromo(Profile* profile) {
-  scoped_refptr<NotificationPromo> notification_promo =
-      NotificationPromo::Create(profile, NULL);
-  notification_promo->InitFromPrefs();
-  return notification_promo->CanShow();
+  NotificationPromo notification_promo(profile);
+  notification_promo.InitFromPrefs();
+  return notification_promo.CanShow();
 }
 
 void PromoResourceService::UnpackWebStoreSignal(

[Dan Beam, 2012/06/04 18:41:04]

if we're not using the tips server, how is any of this used / functional any
more?

[achuithb, 2012/06/04 19:52:59]

Chatted about this; Tips server continues to be used for WebStoreSignal and
LogoSignal. Not sure what the status of those are. Will send an email about this
and deal with this in a separate CL.

     const DictionaryValue& parsed_json) {
   DictionaryValue* topic_dict;
   ListValue* answer_list;
 
   bool is_webstore_active = false;
   bool signal_found = false;
   AppsPromo::PromoData promo_data;
   std::string promo_link = "";
   std::string promo_logo = "";
   int target_builds = 0;
@@ skipping 53 matching lines @@
       // NOTIFICATION_WEB_STORE_PROMO_LOADED notification.
       promo_data.link = GURL(promo_link);
       promo_data.logo = GURL(promo_logo);
       apps_promo_logo_fetcher_.reset(
           new AppsPromoLogoFetcher(profile_, promo_data));
       signal_found = true;
       break;
     }
   }
 
   if (!signal_found) {

[Dan Beam, 2012/06/04 18:41:04]

move comment out and remove curlies, IMO

[achuithb, 2012/06/04 19:52:59]

Hopefully we'll delete this code.

     // If no web store promos target this build, then clear all the prefs.
     AppsPromo::ClearPromo();
   }
 
   AppsPromo::SetWebStoreSupportedForLocale(is_webstore_active);
 
   return;
 }
 
 void PromoResourceService::UnpackLogoSignal(

[Dan Beam, 2012/06/04 18:41:04]

same story here with legacy code probably not being used/working any more

[achuithb, 2012/06/04 19:52:59]

Will send an email, etc.

     const DictionaryValue& parsed_json) {
   DictionaryValue* topic_dict;
   ListValue* answer_list;
   double old_logo_start = 0;
   double old_logo_end = 0;
   double logo_start = 0;
   double logo_end = 0;
 
   // Check for preexisting start and end values.
   if (prefs_->HasPrefPath(prefs::kNtpCustomLogoStart) &&
       prefs_->HasPrefPath(prefs::kNtpCustomLogoEnd)) {
     old_logo_start = prefs_->GetDouble(prefs::kNtpCustomLogoStart);
     old_logo_end = prefs_->GetDouble(prefs::kNtpCustomLogoEnd);
   }
 
   // Check for newly received start and end values.
   if (parsed_json.GetDictionary("topic", &topic_dict)) {
     if (topic_dict->GetList("answers", &answer_list)) {
       std::string logo_start_string = "";
       std::string logo_end_string = "";
       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 logo_signal;
         if (a_dic->GetString("name", &logo_signal)) {
           if (logo_signal == "custom_logo_start") {

[Dan Beam, 2012/06/04 18:41:04]

no curlies

[achuithb, 2012/06/04 19:52:59]

Hopefully delete this

             a_dic->GetString("inproduct", &logo_start_string);
           } else if (logo_signal == "custom_logo_end") {
             a_dic->GetString("inproduct", &logo_end_string);
           }
         }
       }
       if (!logo_start_string.empty() &&
           logo_start_string.length() > 0 &&
           !logo_end_string.empty() &&
           logo_end_string.length() > 0) {
         base::Time start_time;
         base::Time end_time;
         if (base::Time::FromString(logo_start_string.c_str(), &start_time) &&
             base::Time::FromString(logo_end_string.c_str(), &end_time)) {
           logo_start = start_time.ToDoubleT();
           logo_end = end_time.ToDoubleT();
         }
       }
     }
   }
 
   // If logo 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.
   if (!(old_logo_start == logo_start) ||

[Dan Beam, 2012/06/04 18:41:04]

demorgan's law + less ()

[achuithb, 2012/06/04 19:52:59]

hopefully delete this.

       !(old_logo_end == logo_end)) {
     prefs_->SetDouble(prefs::kNtpCustomLogoStart, logo_start);
     prefs_->SetDouble(prefs::kNtpCustomLogoEnd, logo_end);
     content::NotificationService* service =
         content::NotificationService::current();
     service->Notify(chrome::NOTIFICATION_PROMO_RESOURCE_STATE_CHANGED,
                     content::Source<WebResourceService>(this),
                     content::NotificationService::NoDetails());
   }
 }