Immediate notification if active promotion arrives

chrome/browser/web_resource/promo_resource_service.cc

Index: chrome/browser/web_resource/promo_resource_service.cc
diff --git a/chrome/browser/web_resource/promo_resource_service.cc b/chrome/browser/web_resource/promo_resource_service.cc
index fbdeab1e7eec2f62f481167bcfe7315f58fbffe7..25ef64cf04ce41e90c85a7b64f3501ee69181930 100644
--- a/chrome/browser/web_resource/promo_resource_service.cc
+++ b/chrome/browser/web_resource/promo_resource_service.cc
@@ -153,11 +153,12 @@ void PromoResourceService::ScheduleNotification(double promo_start,
     if (ms_until_start > 0)
       PostNotification(ms_until_start);
     if (ms_until_end > 0) {

[achuithb, 2012/06/07 20:34:27]

Maybe also add else here?

[aruslan, 2012/06/07 20:55:40]

Done.

-      PostNotification(ms_until_end);
       if (ms_until_start <= 0) {
         // Notify immediately if time is between start and end.
         PostNotification(0);
       }
+      // Schedule notification at the end of promotion.

[achuithb, 2012/06/07 20:34:27]

Why not use an else block here? I think we are protected by
web_resource_update_scheduled_, but there is no reason to call PostNotification
twice, right?

[aruslan, 2012/06/07 20:55:40]

We should call PostNotification twice -- once with 0 to get an immediate
notification, and the second time with ms_until_end to schedule a future
notification once the promotion is ended, unless I'm missing something.

[achuithb, 2012/06/07 21:31:28]

I see now, that PostNotification sets web_resource_update_scheduled_ to true,
but PromoResourceStateChanged, which gets called from PostNotification, sets it
to false, so the second notification gets scheduled after all. These side
effects are a bit nasty - could you please add some comments about the
orderings, how many notifications are scheduled, etc?

+      PostNotification(ms_until_end);
     }
   }
 }