[GRC] Coordination Unit Graph Observer

services/resource_coordinator/coordination_unit/coordination_unit_manager.cc

 // Copyright 2017 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 "services/resource_coordinator/coordination_unit/coordination_unit_manager.h"
 
-#include <memory>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_graph_observer.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_impl.h"
 #include "services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h"
 #include "services/resource_coordinator/public/cpp/coordination_unit_types.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
@@ skipping 11 matching lines @@
   registry->AddInterface(base::Bind(&CoordinationUnitProviderImpl::Create,
                                     base::Unretained(service_ref_factory),
                                     base::Unretained(this)));
 }
 
 void CoordinationUnitManager::RegisterObserver(
     std::unique_ptr<CoordinationUnitGraphObserver> observer) {
   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