Make AlarmManager::Alarm non copyable.

extensions/browser/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 "extensions/browser/api/alarms/alarm_manager.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 48 matching lines @@
  private:
   content::BrowserContext* browser_context_;
 };
 
 // Creates a TimeDelta from a delay as specified in the API.
 base::TimeDelta TimeDeltaFromDelay(double delay_in_minutes) {
   return base::TimeDelta::FromMicroseconds(delay_in_minutes *
                                            base::Time::kMicrosecondsPerMinute);
 }
 
-std::vector<Alarm> AlarmsFromValue(const std::string extension_id,
-                                   bool is_unpacked,
-                                   const base::ListValue* list) {
-  std::vector<Alarm> alarms;
+AlarmManager::AlarmList AlarmsFromValue(const std::string extension_id,
+                                        bool is_unpacked,
+                                        const base::ListValue* list) {
+  std::vector<std::unique_ptr<Alarm>> alarms;

[Devlin, 2016/06/18 01:50:19]

nitty nit: AlarmManager::AlarmList

[lazyboy, 2016/06/18 02:42:10]

Done.

   for (size_t i = 0; i < list->GetSize(); ++i) {
     const base::DictionaryValue* alarm_dict = nullptr;
-    Alarm alarm;
+    std::unique_ptr<Alarm> alarm(new Alarm());
     if (list->GetDictionary(i, &alarm_dict) &&
-        alarms::Alarm::Populate(*alarm_dict, alarm.js_alarm.get())) {
+        alarms::Alarm::Populate(*alarm_dict, alarm->js_alarm.get())) {
       const base::Value* time_value = nullptr;
       if (alarm_dict->Get(kAlarmGranularity, &time_value))
-        base::GetValueAsTimeDelta(*time_value, &alarm.granularity);
-      alarm.minimum_granularity = base::TimeDelta::FromSecondsD(
+        base::GetValueAsTimeDelta(*time_value, &alarm->granularity);
+      alarm->minimum_granularity = base::TimeDelta::FromSecondsD(
           (is_unpacked ? alarms_api_constants::kDevDelayMinimum
                        : alarms_api_constants::kReleaseDelayMinimum) *
           kSecondsPerMinute);
-      if (alarm.granularity < alarm.minimum_granularity)
-        alarm.granularity = alarm.minimum_granularity;
-      alarms.push_back(alarm);
+      if (alarm->granularity < alarm->minimum_granularity)
+        alarm->granularity = alarm->minimum_granularity;
+      alarms.push_back(std::move(alarm));
     }
   }
   return alarms;
 }
 
 std::unique_ptr<base::ListValue> AlarmsToValue(
-    const std::vector<Alarm>& alarms) {
+    const std::vector<std::unique_ptr<Alarm>>& alarms) {
   std::unique_ptr<base::ListValue> list(new base::ListValue());
   for (size_t i = 0; i < alarms.size(); ++i) {
     std::unique_ptr<base::DictionaryValue> alarm =
-        alarms[i].js_alarm->ToValue();
+        alarms[i]->js_alarm->ToValue();
     alarm->Set(kAlarmGranularity,
-               base::CreateTimeDeltaValue(alarms[i].granularity));
+               base::CreateTimeDeltaValue(alarms[i]->granularity));
     list->Append(std::move(alarm));
   }
   return list;
 }
 
 }  // namespace
 
 // AlarmManager
 
 AlarmManager::AlarmManager(content::BrowserContext* context)
     : browser_context_(context),
       clock_(new base::DefaultClock()),
       delegate_(new DefaultAlarmDelegate(context)),
       extension_registry_observer_(this) {
   extension_registry_observer_.Add(ExtensionRegistry::Get(browser_context_));
 
   StateStore* storage = ExtensionSystem::Get(browser_context_)->state_store();
   if (storage)
     storage->RegisterKey(kRegisteredAlarms);
 }
 
 AlarmManager::~AlarmManager() {
 }
 
 void AlarmManager::AddAlarm(const std::string& extension_id,
-                            const Alarm& alarm,
+                            std::unique_ptr<Alarm> alarm,
                             const AddAlarmCallback& callback) {
-  RunWhenReady(extension_id, base::Bind(&AlarmManager::AddAlarmWhenReady,
-                                        AsWeakPtr(), alarm, callback));
+  RunWhenReady(extension_id,
+               base::Bind(&AlarmManager::AddAlarmWhenReady, AsWeakPtr(),
+                          base::Passed(std::move(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,
+void AlarmManager::AddAlarmWhenReady(std::unique_ptr<Alarm> alarm,
                                      const AddAlarmCallback& callback,
                                      const std::string& extension_id) {
-  AddAlarmImpl(extension_id, alarm);
+  AddAlarmImpl(extension_id, std::move(alarm));
   WriteToStorage(extension_id);
   callback.Run();
 }
 
 void AlarmManager::GetAlarmWhenReady(const std::string& name,
                                      const GetAlarmCallback& callback,
                                      const std::string& extension_id) {
   AlarmIterator it = GetAlarmIterator(extension_id, name);
-  callback.Run(it.first != alarms_.end() ? &*it.second : NULL);
+  callback.Run(it.first != alarms_.end() ? it.second->get() : nullptr);
 }
 
 void AlarmManager::GetAllAlarmsWhenReady(const GetAllAlarmsCallback& callback,
                                          const std::string& extension_id) {
   AlarmMap::iterator list = alarms_.find(extension_id);
   callback.Run(list != alarms_.end() ? &list->second : NULL);
 }
 
 void AlarmManager::RemoveAlarmWhenReady(const std::string& name,
                                         const RemoveAlarmCallback& callback,
@@ skipping 27 matching lines @@
 
 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)
+    if (it->get()->js_alarm->name == name)
       return make_pair(list, it);
   }
 
   return make_pair(alarms_.end(), AlarmList::iterator());
 }
 
 void AlarmManager::SetClockForTesting(base::Clock* clock) {
   clock_.reset(clock);
 }
 
@@ skipping 21 matching lines @@
   // We don't need to reschedule the poll otherwise, because in
   // the worst case we would just poll one extra time.
   if (alarms_.empty()) {
     timer_.Stop();
     next_poll_time_ = base::Time();
   }
 }
 
 void AlarmManager::OnAlarm(AlarmIterator it) {
   CHECK(it.first != alarms_.end());
-  Alarm& alarm = *it.second;
+  Alarm& alarm = *it.second->get();
   std::string extension_id_copy(it.first->first);
   delegate_->OnAlarm(extension_id_copy, alarm);
 
   // Update our scheduled time for the next alarm.
   if (double* period_in_minutes = alarm.js_alarm->period_in_minutes.get()) {
     // Get the timer's delay in JS time (i.e., convert it from minutes to
     // milliseconds).
     double period_in_js_time = *period_in_minutes *
                                base::Time::kMicrosecondsPerMinute /
                                base::Time::kMicrosecondsPerMillisecond;
     // Find out how many periods have transpired since the alarm last went off
     // (it's possible that we missed some).
     int transpired_periods =
         (last_poll_time_.ToJsTime() - alarm.js_alarm->scheduled_time) /
         period_in_js_time;
     // Schedule the alarm for the next period that is in-line with the original
     // scheduling.
     alarm.js_alarm->scheduled_time +=
         period_in_js_time * (transpired_periods + 1);
   } else {
     RemoveAlarmIterator(it);
   }
   WriteToStorage(extension_id_copy);
 }
 
 void AlarmManager::AddAlarmImpl(const std::string& extension_id,
-                                const Alarm& alarm) {
+                                std::unique_ptr<Alarm> alarm) {
   // Override any old alarm with the same name.
   AlarmIterator old_alarm =
-      GetAlarmIterator(extension_id, alarm.js_alarm->name);
+      GetAlarmIterator(extension_id, alarm->js_alarm->name);
   if (old_alarm.first != alarms_.end())
     RemoveAlarmIterator(old_alarm);
 
-  alarms_[extension_id].push_back(alarm);
   base::Time alarm_time =
-      base::Time::FromJsTime(alarm.js_alarm->scheduled_time);
+      base::Time::FromJsTime(alarm->js_alarm->scheduled_time);
+  alarms_[extension_id].push_back(std::move(alarm));
   if (next_poll_time_.is_null() || alarm_time < next_poll_time_)
     SetNextPollTime(alarm_time);
 }
 
 void AlarmManager::WriteToStorage(const std::string& extension_id) {
   StateStore* storage = ExtensionSystem::Get(browser_context_)->state_store();
   if (!storage)
     return;
 
   std::unique_ptr<base::Value> alarms;
   AlarmMap::iterator list = alarms_.find(extension_id);
   if (list != alarms_.end())
     alarms.reset(AlarmsToValue(list->second).release());

[Devlin, 2016/06/18 01:50:19]

nitty nit: since you're here, maybe alarms =
std::move(AlarmsToValue(it->second)); ?

[lazyboy, 2016/06/18 02:42:10]

Done.

   else
-    alarms.reset(AlarmsToValue(std::vector<Alarm>()).release());
+    alarms.reset(AlarmsToValue(AlarmList()).release());
   storage->SetExtensionValue(extension_id, kRegisteredAlarms,
                              std::move(alarms));
 }
 
 void AlarmManager::ReadFromStorage(const std::string& extension_id,
                                    bool is_unpacked,
                                    std::unique_ptr<base::Value> value) {
   base::ListValue* list = NULL;
   if (value.get() && value->GetAsList(&list)) {
-    std::vector<Alarm> alarm_states =
+    std::vector<std::unique_ptr<Alarm>> alarm_states =

[Devlin, 2016/06/18 01:50:19]

AlarmList

[lazyboy, 2016/06/18 02:42:10]

Done.

         AlarmsFromValue(extension_id, is_unpacked, list);
     for (size_t i = 0; i < alarm_states.size(); ++i)
-      AddAlarmImpl(extension_id, alarm_states[i]);
+      AddAlarmImpl(extension_id, std::move(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::SetNextPollTime(const base::Time& time) {
   next_poll_time_ = time;
   timer_.Start(FROM_HERE,
                std::max(base::TimeDelta::FromSeconds(0), time - clock_->Now()),
                this, &AlarmManager::PollAlarms);
 }
 
 void AlarmManager::ScheduleNextPoll() {
   // If there are no alarms, stop the timer.
   if (alarms_.empty()) {
     timer_.Stop();
     next_poll_time_ = base::Time();
     return;
   }
 
   // Find the soonest alarm that is scheduled to run and the smallest
   // granularity of any alarm.
   // alarms_ guarantees that none of its contained lists are empty.
   base::Time soonest_alarm_time = base::Time::FromJsTime(
-      alarms_.begin()->second.begin()->js_alarm->scheduled_time);
+      alarms_.begin()->second.begin()->get()->js_alarm->scheduled_time);
   base::TimeDelta min_granularity = kDefaultMinPollPeriod();
   for (AlarmMap::const_iterator m_it = alarms_.begin(), m_end = alarms_.end();
        m_it != m_end; ++m_it) {
     for (AlarmList::const_iterator l_it = m_it->second.begin();
          l_it != m_it->second.end(); ++l_it) {
       base::Time cur_alarm_time =
-          base::Time::FromJsTime(l_it->js_alarm->scheduled_time);
+          base::Time::FromJsTime(l_it->get()->js_alarm->scheduled_time);
       if (cur_alarm_time < soonest_alarm_time)
         soonest_alarm_time = cur_alarm_time;
-      if (l_it->granularity < min_granularity)
-        min_granularity = l_it->granularity;
+      if (l_it->get()->granularity < min_granularity)
+        min_granularity = l_it->get()->granularity;
       base::TimeDelta cur_alarm_delta = cur_alarm_time - last_poll_time_;
-      if (cur_alarm_delta < l_it->minimum_granularity)
-        cur_alarm_delta = l_it->minimum_granularity;
+      if (cur_alarm_delta < l_it->get()->minimum_granularity)
+        cur_alarm_delta = l_it->get()->minimum_granularity;
       if (cur_alarm_delta < min_granularity)
         min_granularity = cur_alarm_delta;
     }
   }
 
   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.
@@ skipping 13 matching lines @@
   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) <=
+      if (base::Time::FromJsTime(cur_alarm->get()->js_alarm->scheduled_time) <=
           last_poll_time_) {
         OnAlarm(make_pair(cur_extension, cur_alarm));
       }
     }
   }
 
   ScheduleNextPoll();
 }
 
 static void RemoveAllOnUninstallCallback() {
@@ skipping 62 matching lines @@
   if (granularity < min_granularity)
     granularity = min_granularity;
 
   // Check for repetition.
   if (create_info.period_in_minutes.get()) {
     js_alarm->period_in_minutes.reset(
         new double(*create_info.period_in_minutes));
   }
 }
 
-Alarm::Alarm(const Alarm& other) = default;
+Alarm::Alarm(Alarm&& other)
+    : js_alarm(std::move(other.js_alarm)),
+      granularity(std::move(other.granularity)),

[Devlin, 2016/06/18 01:52:45]

nitty nit: if we do keep these move ctors, just copy the TimeDeltas.  They're
just int64s, and we usually even pass them by value, so creating an rvalue ref
is overkill (and maybe in the slightest bit pessimising to performance).

[lazyboy, 2016/06/18 02:42:10]

I started the CL with trying to move Alarm, eventually gave up while changing
AddAlarmImpl() in ReadStorage()

Removed move stuff on Alarm

+      minimum_granularity(std::move(other.minimum_granularity)) {}
+
+Alarm& Alarm::operator=(Alarm&& other) {
+  js_alarm = std::move(other.js_alarm);
+  granularity = std::move(other.granularity);
+  minimum_granularity = std::move(other.minimum_granularity);
+  return *this;
+}
 
 Alarm::~Alarm() {
 }
 
 }  // namespace extensions