Fixing race conditions in chrome.alarms

chrome/browser/extensions/api/alarms/alarm_manager.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/extensions/api/alarms/alarm_manager.h"
 
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 91 matching lines @@
 
   StateStore* storage = ExtensionSystem::Get(profile_)->state_store();
   if (storage)
     storage->RegisterKey(kRegisteredAlarms);
 }
 
 AlarmManager::~AlarmManager() {
 }
 
 void AlarmManager::AddAlarm(const std::string& extension_id,
-                            const Alarm& alarm) {
+                            const Alarm& alarm,
+                            const AddAlarmCallback& callback) {
+  RunWhenReady(extension_id, base::Bind(
+      &AlarmManager::AddAlarmWhenReady, AsWeakPtr(), alarm, callback));
+}
+
+void AlarmManager::GetAlarm(const std::string& extension_id,
+                            const std::string& name,
+                            const GetAlarmCallback& callback) {
+  RunWhenReady(extension_id, base::Bind(
+      &AlarmManager::GetAlarmWhenReady, AsWeakPtr(), name, callback));
+}
+
+void AlarmManager::GetAllAlarms(
+    const std::string& extension_id, const GetAllAlarmsCallback& callback) {
+  RunWhenReady(extension_id, base::Bind(
+      &AlarmManager::GetAllAlarmsWhenReady, AsWeakPtr(), callback));
+}
+
+void AlarmManager::RemoveAlarm(const std::string& extension_id,
+                               const std::string& name,
+                               const RemoveAlarmCallback& callback) {
+  RunWhenReady(extension_id, base::Bind(
+      &AlarmManager::RemoveAlarmWhenReady, AsWeakPtr(), name, callback));
+}
+
+void AlarmManager::RemoveAllAlarms(const std::string& extension_id,
+                                   const RemoveAllAlarmsCallback& callback) {
+  RunWhenReady(extension_id, base::Bind(
+      &AlarmManager::RemoveAllAlarmsWhenReady, AsWeakPtr(), callback));
+}
+
+void AlarmManager::AddAlarmWhenReady(const Alarm& alarm,
+                                     const AddAlarmCallback& callback,
+                                     const std::string& extension_id) {
   AddAlarmImpl(extension_id, alarm);
   WriteToStorage(extension_id);
+  callback.Run();
 }
 
-const Alarm* AlarmManager::GetAlarm(
-    const std::string& extension_id, const std::string& name) {
+void AlarmManager::GetAlarmWhenReady(const std::string& name,
+                                     const GetAlarmCallback& callback,
+                                     const std::string& extension_id) {
   AlarmIterator it = GetAlarmIterator(extension_id, name);
-  if (it.first == alarms_.end())
-    return NULL;
-  return &*it.second;
+  callback.Run(it.first != alarms_.end() ? &*it.second : NULL);
 }
 
-const AlarmManager::AlarmList* AlarmManager::GetAllAlarms(
-    const std::string& extension_id) {
+void AlarmManager::GetAllAlarmsWhenReady(const GetAllAlarmsCallback& callback,
+                                         const std::string& extension_id) {
   AlarmMap::iterator list = alarms_.find(extension_id);
-  if (list == alarms_.end())
-    return NULL;
-  return &list->second;
+  callback.Run(list != alarms_.end() ? &list->second : NULL);
+}
+
+void AlarmManager::RemoveAlarmWhenReady(const std::string& name,
+                                        const RemoveAlarmCallback& callback,
+                                        const std::string& extension_id) {
+  AlarmIterator it = GetAlarmIterator(extension_id, name);
+  if (it.first == alarms_.end()) {
+    callback.Run(false);
+    return;
+  }
+
+  RemoveAlarmIterator(it);
+  WriteToStorage(extension_id);
+  callback.Run(true);
+}
+
+void AlarmManager::RemoveAllAlarmsWhenReady(
+    const RemoveAllAlarmsCallback& callback, const std::string& extension_id) {
+  AlarmMap::iterator list = alarms_.find(extension_id);
+  if (list != alarms_.end()) {
+    // Note: I'm using indices rather than iterators here because
+    // RemoveAlarmIterator will delete the list when it becomes empty.
+    for (size_t i = 0, size = list->second.size(); i < size; ++i)
+      RemoveAlarmIterator(AlarmIterator(list, list->second.begin()));
+
+    CHECK(alarms_.find(extension_id) == alarms_.end());
+    WriteToStorage(extension_id);
+  }
+  callback.Run();
 }
 
 AlarmManager::AlarmIterator AlarmManager::GetAlarmIterator(
     const std::string& extension_id, const std::string& name) {
   AlarmMap::iterator list = alarms_.find(extension_id);
   if (list == alarms_.end())
     return make_pair(alarms_.end(), AlarmList::iterator());
 
   for (AlarmList::iterator it = list->second.begin();
        it != list->second.end(); ++it) {
     if (it->js_alarm->name == name)
       return make_pair(list, it);
   }
 
   return make_pair(alarms_.end(), AlarmList::iterator());
 }
 
-bool AlarmManager::RemoveAlarm(const std::string& extension_id,
-                               const std::string& name) {
-  AlarmIterator it = GetAlarmIterator(extension_id, name);
-  if (it.first == alarms_.end())
-    return false;
-
-  RemoveAlarmIterator(it);
-  WriteToStorage(extension_id);
-  return true;
-}
-
-void AlarmManager::RemoveAllAlarms(const std::string& extension_id) {
-  AlarmMap::iterator list = alarms_.find(extension_id);
-  if (list == alarms_.end())
-    return;
-
-  // Note: I'm using indices rather than iterators here because
-  // RemoveAlarmIterator will delete the list when it becomes empty.
-  for (size_t i = 0, size = list->second.size(); i < size; ++i)
-    RemoveAlarmIterator(AlarmIterator(list, list->second.begin()));
-
-  CHECK(alarms_.find(extension_id) == alarms_.end());
-  WriteToStorage(extension_id);
-}
-
 void AlarmManager::SetClockForTesting(base::Clock* clock) {
   clock_.reset(clock);
 }
 
 static base::LazyInstance<ProfileKeyedAPIFactory<AlarmManager> >
 g_factory = LAZY_INSTANCE_INITIALIZER;
 
 // static
 ProfileKeyedAPIFactory<AlarmManager>* AlarmManager::GetFactoryInstance() {
   return &g_factory.Get();
@@ skipping 58 matching lines @@
   if (list != alarms_.end())
     alarms.reset(AlarmsToValue(list->second).release());
   else
     alarms.reset(AlarmsToValue(std::vector<Alarm>()).release());
   storage->SetExtensionValue(extension_id, kRegisteredAlarms, alarms.Pass());
 }
 
 void AlarmManager::ReadFromStorage(const std::string& extension_id,
                                    scoped_ptr<base::Value> value) {
   base::ListValue* list = NULL;
-  if (!value.get() || !value->GetAsList(&list))
-    return;
+  if (value.get() && value->GetAsList(&list)) {
+    std::vector<Alarm> alarm_states = AlarmsFromValue(list);
+    for (size_t i = 0; i < alarm_states.size(); ++i)
+      AddAlarmImpl(extension_id, alarm_states[i]);
+  }
 
-  std::vector<Alarm> alarm_states = AlarmsFromValue(list);
-  for (size_t i = 0; i < alarm_states.size(); ++i) {
-    AddAlarmImpl(extension_id, alarm_states[i]);
+  ReadyQueue& extension_ready_queue = ready_actions_[extension_id];
+  while (!extension_ready_queue.empty()) {
+    extension_ready_queue.front().Run(extension_id);
+    extension_ready_queue.pop();
   }
+  ready_actions_.erase(extension_id);
 }
 
 void AlarmManager::ScheduleNextPoll() {
   // 0. If there are no alarms, stop the timer.
   if (alarms_.empty()) {
     timer_.Stop();
     return;
   }
 
   // TODO(yoz): Try not to reschedule every single time if we're adding
@@ skipping 20 matching lines @@
 
   base::Time next_poll(last_poll_time_ + min_granularity);
   // If the next alarm is more than min_granularity in the future, wait for it.
   // Otherwise, only poll as often as min_granularity.
   // As a special case, if we've never checked for an alarm before
   // (e.g. during startup), let alarms fire asap.
   if (last_poll_time_.is_null() || next_poll < soonest_alarm_time)
     next_poll = soonest_alarm_time;
 
   // Schedule the poll.
-  next_poll_time_ = next_poll;
+  test_next_poll_time_ = next_poll;
   base::TimeDelta delay = std::max(base::TimeDelta::FromSeconds(0),
                                    next_poll - clock_->Now());
   timer_.Start(FROM_HERE,
                delay,
                this,
                &AlarmManager::PollAlarms);
 }
 
 void AlarmManager::PollAlarms() {
   last_poll_time_ = clock_->Now();
 
   // Run any alarms scheduled in the past. OnAlarm uses vector::erase to remove
   // elements from the AlarmList, and map::erase to remove AlarmLists from the
   // AlarmMap.
   for (AlarmMap::iterator m_it = alarms_.begin(), m_end = alarms_.end();
        m_it != m_end;) {
     AlarmMap::iterator cur_extension = m_it++;
 
     // Iterate (a) backwards so that removing elements doesn't affect
     // upcoming iterations, and (b) with indices so that if the last
     // iteration destroys the AlarmList, I'm not about to use the end
     // iterator that the destruction invalidates.
     for (size_t i = cur_extension->second.size(); i > 0; --i) {
       AlarmList::iterator cur_alarm = cur_extension->second.begin() + i - 1;
       if (base::Time::FromJsTime(cur_alarm->js_alarm->scheduled_time) <=
-          next_poll_time_) {
+          last_poll_time_) {
         OnAlarm(make_pair(cur_extension, cur_alarm));
       }
     }
   }
 
   ScheduleNextPoll();
 }
 
+static void RemoveAllOnUninstallCallback() {}
+
+void AlarmManager::RunWhenReady(
+    const std::string& extension_id, const ReadyAction& action) {
+  ReadyMap::iterator it = ready_actions_.find(extension_id);
+
+  if (it == ready_actions_.end())
+    action.Run(extension_id);
+  else
+    it->second.push(action);
+}
+
 void AlarmManager::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_EXTENSION_LOADED: {
       const Extension* extension =
           content::Details<const Extension>(details).ptr();
       StateStore* storage = ExtensionSystem::Get(profile_)->state_store();
       if (storage) {
+        ready_actions_.insert(
+            ReadyMap::value_type(extension->id(), ReadyQueue()));
         storage->GetExtensionValue(extension->id(), kRegisteredAlarms,
             base::Bind(&AlarmManager::ReadFromStorage,
                        AsWeakPtr(), extension->id()));
       }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
       const Extension* extension =
           content::Details<const Extension>(details).ptr();
-      RemoveAllAlarms(extension->id());
+      RemoveAllAlarms(
+          extension->id(), base::Bind(RemoveAllOnUninstallCallback));
       break;
     }
     default:
       NOTREACHED();
       break;
   }
 }
 
 // AlarmManager::Alarm
 
@@ skipping 32 matching lines @@
   if (create_info.period_in_minutes.get()) {
     js_alarm->period_in_minutes.reset(
         new double(*create_info.period_in_minutes));
   }
 }
 
 Alarm::~Alarm() {
 }
 
 }  // namespace extensions