Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(433)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: services/resource_coordinator/coordination_unit/coordination_unit_graph_observer_unittest.cc

Issue 2942403002: [GRC] Coordination Unit Graph Observer (Closed)
Patch Set: Address reviewer feedback Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « services/resource_coordinator/coordination_unit/coordination_unit_graph_observer.cc ('k') | services/resource_coordinator/coordination_unit/coordination_unit_impl.h » ('j') | services/resource_coordinator/coordination_unit/coordination_unit_manager.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: services/resource_coordinator/coordination_unit/coordination_unit_graph_observer_unittest.cc
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_graph_observer_unittest.cc b/services/resource_coordinator/coordination_unit/coordination_unit_graph_observer_unittest.cc
index 7ad44f721747b13865f1859b7ebffe7e038bc11c..79f298c6a7437a51d12bbd1f3e624f1328ef44f0 100644
--- a/services/resource_coordinator/coordination_unit/coordination_unit_graph_observer_unittest.cc
+++ b/services/resource_coordinator/coordination_unit/coordination_unit_graph_observer_unittest.cc
@@ -25,278 +25,155 @@ namespace {
class CoordinationUnitGraphObserverTest : public CoordinationUnitImplTestBase {
};
-class TestBasicAPICoordinationUnitGraphObserver
- : public CoordinationUnitGraphObserver {
+class TestCoordinationUnitGraphObserver : public CoordinationUnitGraphObserver {
public:
- TestBasicAPICoordinationUnitGraphObserver()
- : TestBasicAPICoordinationUnitGraphObserver(
- CoordinationUnitType::kInvalidType) {}
-
- void OnCoordinationUnitCreatedEvent(
- CoordinationUnitImpl* coordination_unit) override {
- ++on_coordination_unit_created_invocations_;
-
- coordination_unit->ObserveOnChildAddedEvent(this);
- coordination_unit->ObserveOnParentAddedEvent(this);
- coordination_unit->ObserveOnPropertyChangedEvent(this);
- coordination_unit->ObserveOnChildRemovedEvent(this);
- coordination_unit->ObserveOnParentRemovedEvent(this);
- coordination_unit->ObserveOnWillBeDestroyedEvent(this);
- }
-
- size_t on_child_added_listener_invocations() {
- return on_child_added_listener_invocations_;
- }
- size_t on_parent_added_listener_invocations() {
- return on_parent_added_listener_invocations_;
+ TestCoordinationUnitGraphObserver()
+ : on_child_added_count_(0u),
+ on_parent_added_count_(0u),
+ on_coordination_unit_created_count_(0u),
+ on_property_changed_count_(0u),
+ on_child_removed_count_(0u),
+ on_parent_removed_count_(0u),
+ on_coordination_unit_will_be_destroyed_count_(0u) {}
+
+ size_t on_child_added_count() { return on_child_added_count_; }
+ size_t on_parent_added_count() { return on_parent_added_count_; }
+ size_t on_coordination_unit_created_count() {
+ return on_coordination_unit_created_count_;
}
- size_t on_coordination_unit_created_invocations() {
- return on_coordination_unit_created_invocations_;
+ size_t on_property_changed_count() { return on_property_changed_count_; }
+ size_t on_child_removed_count() { return on_child_removed_count_; }
+ size_t on_parent_removed_count() { return on_parent_removed_count_; }
+ size_t on_coordination_unit_will_be_destroyed_count() {
+ return on_coordination_unit_will_be_destroyed_count_;
}
- size_t on_property_changed_listener_invocations() {
- return on_property_changed_listener_invocations_;
- }
- size_t on_child_removed_listener_invocations() {
- return on_child_removed_listener_invocations_;
- }
- size_t on_parent_removed_listener_invocations() {
- return on_parent_removed_listener_invocations_;
+
+ // Overridden from CoordinationUnitGraphObserver.
+ bool ShouldObserve(const CoordinationUnitImpl* coordination_unit) override {
+ return coordination_unit->id().type == CoordinationUnitType::kFrame;
}
- size_t on_will_be_destroyed_listener_invocations() {
- return on_will_be_destroyed_listener_invocations_;
+ void OnCoordinationUnitCreated(
+ const CoordinationUnitImpl* coordination_unit) override {
+ ++on_coordination_unit_created_count_;
}
-
- void OnChildAddedEvent(
+ void OnChildAdded(
const CoordinationUnitImpl* coordination_unit,
const CoordinationUnitImpl* child_coordination_unit) override {
- ++on_child_added_listener_invocations_;
+ ++on_child_added_count_;
}
- void OnParentAddedEvent(
+ void OnParentAdded(
const CoordinationUnitImpl* coordination_unit,
const CoordinationUnitImpl* parent_coordination_unit) override {
- ++on_parent_added_listener_invocations_;
+ ++on_parent_added_count_;
}
- void OnPropertyChangedEvent(const CoordinationUnitImpl* coordination_unit,
- mojom::PropertyType property) override {
- ++on_property_changed_listener_invocations_;
+ void OnPropertyChanged(const CoordinationUnitImpl* coordination_unit,
+ mojom::PropertyType property) override {
+ ++on_property_changed_count_;
}
- void OnChildRemovedEvent(
+ void OnChildRemoved(
const CoordinationUnitImpl* coordination_unit,
const CoordinationUnitImpl* former_child_coordination_unit) override {
- ++on_child_removed_listener_invocations_;
+ ++on_child_removed_count_;
}
- void OnParentRemovedEvent(
+ void OnParentRemoved(
const CoordinationUnitImpl* coordination_unit,
const CoordinationUnitImpl* former_parent_coordination_unit) override {
- ++on_parent_removed_listener_invocations_;
+ ++on_parent_removed_count_;
}
- void OnWillBeDestroyedEvent(
+ void OnCoordinationUnitWillBeDestroyed(
const CoordinationUnitImpl* coordination_unit) override {
- ++on_will_be_destroyed_listener_invocations_;
+ ++on_coordination_unit_will_be_destroyed_count_;
}
- protected:
- explicit TestBasicAPICoordinationUnitGraphObserver(
- CoordinationUnitType filter)
- : CoordinationUnitGraphObserver(filter),
- on_child_added_listener_invocations_(0u),
- on_parent_added_listener_invocations_(0u),
- on_coordination_unit_created_invocations_(0u),
- on_property_changed_listener_invocations_(0u),
- on_child_removed_listener_invocations_(0u),
- on_parent_removed_listener_invocations_(0u),
- on_will_be_destroyed_listener_invocations_(0u) {}
-
- size_t on_child_added_listener_invocations_;
- size_t on_parent_added_listener_invocations_;
- size_t on_coordination_unit_created_invocations_;
- size_t on_property_changed_listener_invocations_;
- size_t on_child_removed_listener_invocations_;
- size_t on_parent_removed_listener_invocations_;
- size_t on_will_be_destroyed_listener_invocations_;
-};
-
-class TestFilterAPICoordinationUnitGraphObserver
- : public TestBasicAPICoordinationUnitGraphObserver {
- public:
- explicit TestFilterAPICoordinationUnitGraphObserver(
- CoordinationUnitType filter)
- : TestBasicAPICoordinationUnitGraphObserver(filter) {}
-
- void OnCoordinationUnitCreatedEvent(
- CoordinationUnitImpl* coordination_unit) override {
- ++on_coordination_unit_created_invocations_;
-
- coordination_unit->ObserveOnChildAddedEvent(this,
- CoordinationUnitType::kFrame);
- coordination_unit->ObserveOnParentAddedEvent(
- this, CoordinationUnitType::kProcess);
- // TODO(matthalp) Use property mojom::PropertyType enum once it is created.
- // Currently the only enum is mojom::PropertyType::kTest which is mean to
- // be filtered out in the test this observer class is used in.
- coordination_unit->ObserveOnPropertyChangedEvent(
- this, static_cast<mojom::PropertyType>(1));
- coordination_unit->ObserveOnChildRemovedEvent(
- this, CoordinationUnitType::kNavigation);
- coordination_unit->ObserveOnParentRemovedEvent(
- this, CoordinationUnitType::kWebContents);
- coordination_unit->ObserveOnWillBeDestroyedEvent(this);
- }
+ private:
+ size_t on_child_added_count_;
+ size_t on_parent_added_count_;
+ size_t on_coordination_unit_created_count_;
+ size_t on_property_changed_count_;
+ size_t on_child_removed_count_;
+ size_t on_parent_removed_count_;
+ size_t on_coordination_unit_will_be_destroyed_count_;
};
} // namespace
TEST_F(CoordinationUnitGraphObserverTest, CallbacksInvokedNoFilters) {
- TestBasicAPICoordinationUnitGraphObserver* observer =
- new TestBasicAPICoordinationUnitGraphObserver();
-
- // The observer will be deleted after this test executes when the
- // CoordinationUnitManager object goes out of scope and destructs.
- coordination_unit_manager()->RegisterObserver(
- std::unique_ptr<TestBasicAPICoordinationUnitGraphObserver>(observer));
-
- // The CoordinationUnit types are intentionally different to make
- // sure filtering, which is not disabled, does not occur.
- CoordinationUnitID parent_cu_id(CoordinationUnitType::kWebContents,
- std::string());
- CoordinationUnitID child_cu_id(CoordinationUnitType::kFrame, std::string());
-
- std::unique_ptr<CoordinationUnitImpl> parent_coordination_unit =
- coordination_unit_factory::CreateCoordinationUnit(
- parent_cu_id, service_context_ref_factory()->CreateRef());
- std::unique_ptr<CoordinationUnitImpl> child_coordination_unit =
- coordination_unit_factory::CreateCoordinationUnit(
- child_cu_id, service_context_ref_factory()->CreateRef());
-
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
- parent_coordination_unit.get());
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
- child_coordination_unit.get());
-
- parent_coordination_unit->AddChild(child_coordination_unit->id());
- parent_coordination_unit->RemoveChild(child_coordination_unit->id());
-
- parent_coordination_unit->SetProperty(mojom::PropertyType::kTest,
- base::Value(42));
+ EXPECT_TRUE(coordination_unit_manager().observers().empty());
+ coordination_unit_manager().RegisterObserver(
+ base::MakeUnique<TestCoordinationUnitGraphObserver>());
- child_coordination_unit->WillBeDestroyed();
- parent_coordination_unit->WillBeDestroyed();
+ EXPECT_EQ(1u, coordination_unit_manager().observers().size());
+ TestCoordinationUnitGraphObserver* observer =
+ static_cast<TestCoordinationUnitGraphObserver*>(
+ coordination_unit_manager().observers()[0].get());
- EXPECT_EQ(1u, observer->on_child_added_listener_invocations());
- EXPECT_EQ(1u, observer->on_child_removed_listener_invocations());
- EXPECT_EQ(2u, observer->on_coordination_unit_created_invocations());
- EXPECT_EQ(1u, observer->on_parent_added_listener_invocations());
- EXPECT_EQ(1u, observer->on_parent_removed_listener_invocations());
- EXPECT_EQ(1u, observer->on_property_changed_listener_invocations());
- EXPECT_EQ(2u, observer->on_will_be_destroyed_listener_invocations());
-}
-
-TEST_F(CoordinationUnitGraphObserverTest, CallbacksInvokedWithoutFilters) {
- TestFilterAPICoordinationUnitGraphObserver* observer =
- new TestFilterAPICoordinationUnitGraphObserver(
- CoordinationUnitType::kFrame);
- coordination_unit_manager()->RegisterObserver(
- std::unique_ptr<TestFilterAPICoordinationUnitGraphObserver>(observer));
-
- // The CoordinationUnit types are intentionally different to test
- // that filtering is working correctly.
CoordinationUnitID process_cu_id(CoordinationUnitType::kProcess,
std::string());
- CoordinationUnitID tab_cu_id(CoordinationUnitType::kWebContents,
- std::string());
CoordinationUnitID root_frame_cu_id(CoordinationUnitType::kFrame,
std::string());
CoordinationUnitID frame_cu_id(CoordinationUnitType::kFrame, std::string());
- CoordinationUnitID navigation_cu_id(CoordinationUnitType::kNavigation,
- std::string());
std::unique_ptr<CoordinationUnitImpl> process_coordination_unit =
coordination_unit_factory::CreateCoordinationUnit(
process_cu_id, service_context_ref_factory()->CreateRef());
- std::unique_ptr<CoordinationUnitImpl> tab_coordination_unit =
- coordination_unit_factory::CreateCoordinationUnit(
- tab_cu_id, service_context_ref_factory()->CreateRef());
std::unique_ptr<CoordinationUnitImpl> root_frame_coordination_unit =
coordination_unit_factory::CreateCoordinationUnit(
root_frame_cu_id, service_context_ref_factory()->CreateRef());
std::unique_ptr<CoordinationUnitImpl> frame_coordination_unit =
coordination_unit_factory::CreateCoordinationUnit(
frame_cu_id, service_context_ref_factory()->CreateRef());
- std::unique_ptr<CoordinationUnitImpl> navigation_coordination_unit =
- coordination_unit_factory::CreateCoordinationUnit(
- navigation_cu_id, service_context_ref_factory()->CreateRef());
- // Should only invoke the OnCoordinationUnitCreated listener for
- // the root_frame and frame CoordinationUnits because the observer
- // filter is set to CoordinationUnitType::kFrame.
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
+ coordination_unit_manager().OnCoordinationUnitCreated(
process_coordination_unit.get());
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
- tab_coordination_unit.get());
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
+ coordination_unit_manager().OnCoordinationUnitCreated(
root_frame_coordination_unit.get());
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
+ coordination_unit_manager().OnCoordinationUnitCreated(
frame_coordination_unit.get());
- coordination_unit_manager()->NotifyObserversCoordinationUnitCreated(
- navigation_coordination_unit.get());
-
- // Test AddChild filtering. The AddChild filter for the frame
- // CoordinationUnit has been set to only execute when the child
- // is a CoordinationUnitType::kFrame so the callback will execute.
- root_frame_coordination_unit->AddChild(frame_coordination_unit->id());
- // The child is CoordinationUnitType::kNavigation so the callback
- // should not be executed. The navigation Coordination Unit is not
- // actually a child of a frame in practice, but is being used here to
- // check filtering on OnChildAdded.
- root_frame_coordination_unit->AddChild(navigation_coordination_unit->id());
+ EXPECT_EQ(3u, observer->on_coordination_unit_created_count());
- // Test AddParent filtering. The AddParent filter for the frame
- // CoordinationUnit has been set to only execute when the parent
- // is a CoordinationUnitType::kProcess so the callback will execute.
- // Note that AddParent is called within Add child.
+ // The registered observer will only observe the events that happen to
+ // |root_frame_coordination_unit| and |frame_coordination_unit| because
+ // they are CoordinationUnitType::kFrame.
+ // OnAddParent will called for |root_frame_coordination_unit|.
process_coordination_unit->AddChild(root_frame_coordination_unit->id());
- // The parent is CoordinationUnitType::kFrame so the callback
- // should not be executed.
- tab_coordination_unit->AddChild(root_frame_coordination_unit->id());
-
- // Test RemoveChild filtering. The RemoveChild filter for the frame
- // CoordinationUnit has been set to only execute when the child
- // is a CoordinationUnitType::kFrame so the callback will execute.
- root_frame_coordination_unit->RemoveChild(frame_coordination_unit->id());
- // The child is CoordinationUnitType::kNavigation so the callback
- // should not be executed. The navigation Coordination Unit is not
- // actually a child of a frame in practice, but is being used here to
- // check filtering on OnRemoveChild.
- root_frame_coordination_unit->RemoveChild(navigation_coordination_unit->id());
+ // OnAddParent will called for |frame_coordination_unit|.
+ process_coordination_unit->AddChild(frame_coordination_unit->id());
+ // OnAddChild will called for |root_frame_coordination_unit| and
+ // OnAddParent will called for |frame_coordination_unit|.
+ root_frame_coordination_unit->AddChild(frame_coordination_unit->id());
+ EXPECT_EQ(1u, observer->on_child_added_count());
+ EXPECT_EQ(3u, observer->on_parent_added_count());
- // Test RemoveParent filtering. The RemoveParent filter for the frame
- // CoordinationUnit has been set to only execute when the parent
- // is a CoordinationUnitType::kProcess so the callback will execute.
- // Note that RemoveParent is called within Remove child.
+ // The registered observer will only observe the events that happen to
+ // |root_frame_coordination_unit| and |frame_coordination_unit| because
+ // they are CoordinationUnitType::kFrame.
+ // OnRemoveParent will called for |root_frame_coordination_unit|.
process_coordination_unit->RemoveChild(root_frame_coordination_unit->id());
- // The parent is CoordinationUnitType::kFrame so the callback
- // should not be executed.
- tab_coordination_unit->RemoveChild(root_frame_coordination_unit->id());
+ // OnRemoveParent will called for |frame_coordination_unit|.
+ process_coordination_unit->RemoveChild(frame_coordination_unit->id());
+ // OnRemoveChild will called for |root_frame_coordination_unit| and
+ // OnRemoveParent will called for |frame_coordination_unit|.
+ root_frame_coordination_unit->AddChild(frame_coordination_unit->id());
+ EXPECT_EQ(1u, observer->on_child_removed_count());
+ EXPECT_EQ(3u, observer->on_parent_removed_count());
- // TODO(matthalp) Implement this another SetProperty once an additional
- // mojom::PropertyType has been implemented so that the SetProperty
- // filtering API can be tested.
+ // The registered observer will only observe the events that happen to
+ // |root_frame_coordination_unit| and |frame_coordination_unit| because
+ // they are CoordinationUnitType::kFrame, so OnPropertyChanged
+ // will only be called for |root_frame_coordination_unit|.
root_frame_coordination_unit->SetProperty(mojom::PropertyType::kTest,
base::Value(42));
+ process_coordination_unit->SetProperty(mojom::PropertyType::kTest,
+ base::Value(42));
+ EXPECT_EQ(1u, observer->on_property_changed_count());
- frame_coordination_unit->WillBeDestroyed();
- navigation_coordination_unit->WillBeDestroyed();
- process_coordination_unit->WillBeDestroyed();
- root_frame_coordination_unit->WillBeDestroyed();
- tab_coordination_unit->WillBeDestroyed();
-
- EXPECT_EQ(1u, observer->on_child_added_listener_invocations());
- EXPECT_EQ(1u, observer->on_child_removed_listener_invocations());
- EXPECT_EQ(2u, observer->on_coordination_unit_created_invocations());
- EXPECT_EQ(1u, observer->on_parent_added_listener_invocations());
- EXPECT_EQ(1u, observer->on_parent_removed_listener_invocations());
- EXPECT_EQ(0u, observer->on_property_changed_listener_invocations());
- EXPECT_EQ(2u, observer->on_will_be_destroyed_listener_invocations());
+ coordination_unit_manager().OnCoordinationUnitWillBeDestroyed(
+ process_coordination_unit.get());
+ coordination_unit_manager().OnCoordinationUnitWillBeDestroyed(
+ root_frame_coordination_unit.get());
+ coordination_unit_manager().OnCoordinationUnitWillBeDestroyed(
+ frame_coordination_unit.get());
+ EXPECT_EQ(3u, observer->on_coordination_unit_will_be_destroyed_count());
}
} // namespace resource_coordinator
« no previous file with comments | « services/resource_coordinator/coordination_unit/coordination_unit_graph_observer.cc ('k') | services/resource_coordinator/coordination_unit/coordination_unit_impl.h » ('j') | services/resource_coordinator/coordination_unit/coordination_unit_manager.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698