[GRC] Coordination Unit Graph Observer

services/resource_coordinator/coordination_unit/coordination_unit_manager.cc

Index: services/resource_coordinator/coordination_unit/coordination_unit_manager.cc
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_manager.cc b/services/resource_coordinator/coordination_unit/coordination_unit_manager.cc
index 2a31c2551deb93332a0ca0f7bdad1ca4fc431531..37d0aebadbfe6b17f6539e1a200f7e07bab0359e 100644
--- a/services/resource_coordinator/coordination_unit/coordination_unit_manager.cc
+++ b/services/resource_coordinator/coordination_unit/coordination_unit_manager.cc
@@ -4,7 +4,6 @@
 
 #include "services/resource_coordinator/coordination_unit/coordination_unit_manager.h"
 
-#include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ -36,18 +35,23 @@ void CoordinationUnitManager::RegisterObserver(
   observers_.push_back(std::move(observer));
 }
 
-void CoordinationUnitManager::NotifyObserversCoordinationUnitCreated(
+void CoordinationUnitManager::OnCoordinationUnitCreated(
     CoordinationUnitImpl* coordination_unit) {
   for (auto& observer : observers_) {
     if (observer->ShouldObserve(coordination_unit)) {
-      observer->OnCoordinationUnitCreatedEvent(coordination_unit);
+      coordination_unit->AddObserver(observer.get());
+      observer->OnCoordinationUnitCreated(coordination_unit);
     }
   }
 }
 
-void CoordinationUnitManager::NotifyObserversCoordinationUnitWillBeDestroyed(
+void CoordinationUnitManager::OnCoordinationUnitWillBeDestroyed(
     CoordinationUnitImpl* coordination_unit) {
-  coordination_unit->WillBeDestroyed();
+  for (auto& observer : observers_) {
+    if (observer->ShouldObserve(coordination_unit)) {
+      observer->OnCoordinationUnitWillBeDestroyed(coordination_unit);

[matthalp, 2017/06/20 21:02:09]

I advocate moving the OnCoordinationUnitWillBeDestroyed logic back into the
CoordinationUnit. Doing the if (observer->ShouldObserver(coordinationUnit) check
here requires that ShouldObserve is pure in that it gives the same output every
time that coordination_unit instance is passed into it.

While I think that most of that code will be that way, there is no guarantee for
that to be the case. For example an observer could decide to do

bool ShouldObserver(...) {
  return tracked_coordination_units_ < 5;
}

void CoordinationUnitCreated(...) {
  ++tracked_coordination_units_;
}

In this case, if a CU was one of the first five CUs created it would be tracked,
but if more than five were created when this CU is destroyed then
OnCoordinationUnitWillBeDestroyed would not be invoked.

I know this is a pathological example, but you never know.

+    }
+  }
 }
 
 }  // namespace resource_coordinator