Hook up views/widgets to AshDevToolsDOMAgent

ash/common/devtools/ash_devtools_dom_agent.cc

Index: ash/common/devtools/ash_devtools_dom_agent.cc
diff --git a/ash/common/devtools/ash_devtools_dom_agent.cc b/ash/common/devtools/ash_devtools_dom_agent.cc
index a8dcd7dcd2e96efc598637b552f3e826d9808e90..6f0680a922ffb73c3029331cb818231f91347e47 100644
--- a/ash/common/devtools/ash_devtools_dom_agent.cc
+++ b/ash/common/devtools/ash_devtools_dom_agent.cc
@@ -66,6 +66,19 @@ WmWindow* FindPreviousSibling(WmWindow* window) {
   return it == siblings.begin() ? nullptr : *std::prev(it);
 }
 
+views::View* FindPreviousSibling(views::View* view) {
+  views::View* parent = view->parent();
+  int view_index = -1;
+  for (int i = 0, count = parent->child_count(); i < count; i++) {
+    if (view == parent->child_at(i)) {
+      view_index = i;
+      break;
+    }
+  }
+  DCHECK(view_index >= 0);

[sadrul, 2016/11/25 18:27:48]

DCHECK_GE

[Sarmad Hashmi, 2016/11/25 23:24:34]

Done.

+  return view_index == 0 ? nullptr : parent->child_at(view_index - 1);
+}
+
 }  // namespace
 
 AshDevToolsDOMAgent::AshDevToolsDOMAgent(ash::WmShell* shell) : shell_(shell) {
@@ -73,7 +86,7 @@ AshDevToolsDOMAgent::AshDevToolsDOMAgent(ash::WmShell* shell) : shell_(shell) {
 }
 
 AshDevToolsDOMAgent::~AshDevToolsDOMAgent() {
-  RemoveObserverFromAllWindows();
+  RemoveObservers();
 }
 
 // DOM::Backend implementation
@@ -120,6 +133,28 @@ void AshDevToolsDOMAgent::OnWindowStackingChanged(WmWindow* window) {
   AddWindowTree(window);
 }
 
+// views::WidgetRemovalsObserver implementation
+void AshDevToolsDOMAgent::OnWillRemoveView(views::Widget* widget,
+                                           views::View* view) {
+  if (view == widget->GetRootView())
+    RemoveViewTree(view, nullptr, true);
+}
+
+// views::ViewObserver implementation
+void AshDevToolsDOMAgent::OnChildViewRemoved(views::View* view,
+                                             views::View* parent) {
+  RemoveViewTree(view, parent, true);
+}
+
+void AshDevToolsDOMAgent::OnChildViewAdded(views::View* view) {
+  AddViewTree(view);
+}
+
+void AshDevToolsDOMAgent::OnChildViewReordered(views::View* view) {
+  RemoveViewTree(view, view->parent(), false);
+  AddViewTree(view);
+}
+
 WmWindow* AshDevToolsDOMAgent::GetWindowFromNodeId(int nodeId) {
   return node_id_to_window_map_.count(nodeId) ? node_id_to_window_map_[nodeId]
                                               : nullptr;
@@ -184,7 +219,8 @@ std::unique_ptr<DOM::Node> AshDevToolsDOMAgent::BuildTreeForRootWidget(
   children->addItem(BuildTreeForView(widget->GetRootView()));
   std::unique_ptr<ui::devtools::protocol::DOM::Node> node =
       BuildNode("Widget", GetAttributes(widget), std::move(children));
-  // TODO(mhashmi): Add WidgetRemovalsObserver here
+  if (!widget->HasRemovalsObserver(this))

[sadrul, 2016/11/25 18:27:48]

Do we need to check here? If this ever returns true, that would mean the DCHECK
above in line 217 would trip, right?

[Sarmad Hashmi, 2016/11/25 23:24:34]

Not necessarily. If this returns true, then that means |remove_observer| was
false when calling the remove methods. This is so that when methods like
OnWindowStackingChanged or OnChildViewReordered are called, we don't remove and
add the observer in the callbacks (which would result in endless calls to the
observer).

So, widget->HasRemovalsObserver(this) or (view|window)->HasObserver(this) would
return true if we ever have a case where the node is being removed and added
within the same callback (observer method).

+    widget->AddRemovalsObserver(this);
   widget_to_node_id_map_[widget] = node->getNodeId();
   node_id_to_widget_map_[node->getNodeId()] = widget;
   return node;
@@ -198,7 +234,8 @@ std::unique_ptr<DOM::Node> AshDevToolsDOMAgent::BuildTreeForView(
     children->addItem(BuildTreeForView(view->child_at(i)));
   std::unique_ptr<ui::devtools::protocol::DOM::Node> node =
       BuildNode("View", GetAttributes(view), std::move(children));
-  // TODO(mhashmi): Add ViewObserver here
+  if (!view->HasObserver(this))

[sadrul, 2016/11/25 18:27:48]

ditto

[Sarmad Hashmi, 2016/11/25 23:24:34]

Same as above.

+    view->AddObserver(this);
   view_to_node_id_map_[view] = node->getNodeId();
   node_id_to_view_map_[node->getNodeId()] = view;
   return node;
@@ -240,6 +277,7 @@ void AshDevToolsDOMAgent::RemoveWindowNode(WmWindow* window,
 
   if (remove_observer)
     window->RemoveObserver(this);
+
   node_id_to_window_map_.erase(node_id_to_window_it);
   window_to_node_id_map_.erase(window_to_node_id_it);
   frontend()->childNodeRemoved(parent_id, node_id);
@@ -263,8 +301,8 @@ void AshDevToolsDOMAgent::RemoveWidgetNode(views::Widget* widget,
   int parent_id =
       GetNodeIdFromWindow(WmLookup::Get()->GetWindowForWidget(widget));
 
-  // TODO(mhashmi): Add WidgetRemovalsObserver and remove it here based on
-  // |remove_observer|
+  if (remove_observer)
+    widget->RemoveRemovalsObserver(this);
 
   NodeIdToWidgetMap::iterator node_id_to_widget_it =
       node_id_to_widget_map_.find(node_id);
@@ -275,6 +313,15 @@ void AshDevToolsDOMAgent::RemoveWidgetNode(views::Widget* widget,
   frontend()->childNodeRemoved(parent_id, node_id);
 }
 
+void AshDevToolsDOMAgent::AddViewTree(views::View* view) {
+  DCHECK(view_to_node_id_map_.count(view->parent()));
+  views::View* prev_sibling = FindPreviousSibling(view);
+  frontend()->childNodeInserted(
+      view_to_node_id_map_[view->parent()],
+      prev_sibling ? view_to_node_id_map_[prev_sibling] : 0,
+      BuildTreeForView(view));
+}
+
 void AshDevToolsDOMAgent::RemoveViewTree(views::View* view,
                                          views::View* parent,
                                          bool remove_observer) {
@@ -298,8 +345,8 @@ void AshDevToolsDOMAgent::RemoveViewNode(views::View* view,
   else  // views::RootView
     parent_id = GetNodeIdFromWidget(view->GetWidget());
 
-  // TODO(mhashmi): Add ViewObserver and remove it here based on
-  // |remove_observer|
+  if (remove_observer)
+    view->RemoveObserver(this);
 
   NodeIdToViewMap::iterator node_id_to_view_it =
       node_id_to_view_map_.find(node_id);
@@ -310,13 +357,17 @@ void AshDevToolsDOMAgent::RemoveViewNode(views::View* view,
   frontend()->childNodeRemoved(parent_id, node_id);
 }
 
-void AshDevToolsDOMAgent::RemoveObserverFromAllWindows() {
+void AshDevToolsDOMAgent::RemoveObservers() {
   for (auto& pair : window_to_node_id_map_)
     pair.first->RemoveObserver(this);
+  for (auto& pair : widget_to_node_id_map_)
+    pair.first->RemoveRemovalsObserver(this);
+  for (auto& pair : view_to_node_id_map_)
+    pair.first->RemoveObserver(this);
 }
 
 void AshDevToolsDOMAgent::Reset() {
-  RemoveObserverFromAllWindows();
+  RemoveObservers();
   window_to_node_id_map_.clear();
   widget_to_node_id_map_.clear();
   view_to_node_id_map_.clear();