PolicyManager: Call all the ValueChanged() on the same main loop call.

This patch calls the ValueChanged() method on all the observers from
the same main loop calling, fixing a possible free after use if the
observer gets removed and deleted between the NotifyValueChanged()
call and the ValueChanged() calls.

BUG=chromium:355132
TEST=Unittest added.

Change-Id: I6365c3b86fe0b85502bfa81bbd18a86c55caa009
Reviewed-on: https://chromium-review.googlesource.com/192526
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/policy_manager/variable.h b/policy_manager/variable.h
index 22cdd17..ab19f72 100644
--- a/policy_manager/variable.h
+++ b/policy_manager/variable.h
@@ -102,10 +102,8 @@
 
   // Calls ValueChanged on all the observers.
   void NotifyValueChanged() {
-    for (auto& observer : observer_list_) {
-      RunFromMainLoop(base::Bind(&BaseVariable::ObserverInterface::ValueChanged,
-                                 base::Unretained(observer), this));
-    }
+    RunFromMainLoop(base::Bind(&BaseVariable::OnValueChangedNotification,
+                               base::Unretained(this)));
   }
 
  private:
@@ -113,6 +111,7 @@
   friend class PmBaseVariableTest;
   FRIEND_TEST(PmBaseVariableTest, RepeatedObserverTest);
   FRIEND_TEST(PmBaseVariableTest, NotifyValueChangedTest);
+  FRIEND_TEST(PmBaseVariableTest, NotifyValueRemovesObserversTest);
 
   BaseVariable(const std::string& name, VariableMode mode,
                base::TimeDelta poll_interval)
@@ -120,6 +119,22 @@
       poll_interval_(mode == kVariableModePoll ?
                      poll_interval : base::TimeDelta()) {}
 
+  void OnValueChangedNotification() {
+    // A ValueChanged() method can change the list of observers, for example
+    // removing itself and invalidating the iterator, so we create a snapshot
+    // of the observers first. Also, to support the case when *another* observer
+    // is removed, we check for them.
+    std::list<BaseVariable::ObserverInterface*> observer_list_copy(
+        observer_list_);
+
+    for (auto& observer : observer_list_copy) {
+      if (std::find(observer_list_.begin(), observer_list_.end(), observer) !=
+          observer_list_.end()) {
+        observer->ValueChanged(this);
+      }
+    }
+  }
+
   // The default PollInterval in minutes.
   static constexpr int kDefaultPollMinutes = 5;