ui_devtools: optimize view tree updating logic

Currently when view tree is updating, we visit each view in this subtree
and for each view we rebuild the subtree with that view as root and
recurse. This causes very complicated view tree, e.g. ChromeOS launcher
to flood the HTTP websocket server and overrun the buffer.

Now we avoid rebuilding subtree if we already visited it, and only send
view tree update on the root view which covers the whole subtree.

Bug: 894107
Test: locally build and run devtools with ChromeOS launcher
Change-Id: I53c87813729eb4983ce613628ae2b83e86adcd30
Reviewed-on: https://chromium-review.googlesource.com/c/1279357Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600094}

components/ui_devtools/dom_agent.cc

@@ -5,6 +5,8 @@
 #include "components/ui_devtools/dom_agent.h"
 
 #include <memory>
+#include <string>
+#include <utility>
 
 #include "components/ui_devtools/devtools_server.h"
 #include "components/ui_devtools/root_element.h"
@@ -12,7 +14,9 @@
 
 namespace ui_devtools {
 
-using namespace ui_devtools::protocol;
+using ui_devtools::protocol::Array;
+using ui_devtools::protocol::DOM::Node;
+using ui_devtools::protocol::Response;
 
 DOMAgent::DOMAgent() {}
 
@@ -25,7 +29,7 @@ Response DOMAgent::disable() {
   return Response::OK();
 }
 
-Response DOMAgent::getDocument(std::unique_ptr<DOM::Node>* out_root) {
+Response DOMAgent::getDocument(std::unique_ptr<Node>* out_root) {
   *out_root = BuildInitialTree();
   return Response::OK();
 }
@@ -45,17 +49,28 @@ void DOMAgent::OnUIElementAdded(UIElement* parent, UIElement* child) {
     node_id_to_ui_element_[child->node_id()] = child;
     return;
   }
-  // If tree is being built, don't add child to dom tree again.
-  if (is_building_tree_)
-    return;
+
   DCHECK(node_id_to_ui_element_.count(parent->node_id()));
 
+  auto* current_parent = parent;
+  while (current_parent) {
+    if (current_parent->is_updating()) {
+      // One of the parents is updating, so no need to update here.
+      return;
+    }
+    current_parent = current_parent->parent();
+  }
+
+  child->set_is_updating(true);
+
   const auto& children = parent->children();
   auto iter = std::find(children.begin(), children.end(), child);
   int prev_node_id =
       (iter == children.end() - 1) ? 0 : (*std::next(iter))->node_id();
   frontend()->childNodeInserted(parent->node_id(), prev_node_id,
                                 BuildTreeForUIElement(child));
+
+  child->set_is_updating(false);
 }
 
 void DOMAgent::OnUIElementReordered(UIElement* parent, UIElement* child) {
@@ -107,27 +122,26 @@ int DOMAgent::GetParentIdOfNodeId(int node_id) const {
 
 // TODO(mhashmi): Make ids reusable
 
-std::unique_ptr<DOM::Node> DOMAgent::BuildNode(
+std::unique_ptr<Node> DOMAgent::BuildNode(
     const std::string& name,
     std::unique_ptr<Array<std::string>> attributes,
-    std::unique_ptr<Array<DOM::Node>> children,
+    std::unique_ptr<Array<Node>> children,
     int node_ids) {
   constexpr int kDomElementNodeType = 1;
-  std::unique_ptr<DOM::Node> node = DOM::Node::create()
-                                        .setNodeId(node_ids)
-                                        .setBackendNodeId(node_ids)
-                                        .setNodeName(name)
-                                        .setNodeType(kDomElementNodeType)
-                                        .setAttributes(std::move(attributes))
-                                        .build();
+  std::unique_ptr<Node> node = Node::create()
+                                   .setNodeId(node_ids)
+                                   .setBackendNodeId(node_ids)
+                                   .setNodeName(name)
+                                   .setNodeType(kDomElementNodeType)
+                                   .setAttributes(std::move(attributes))
+                                   .build();
   node->setChildNodeCount(static_cast<int>(children->length()));
   node->setChildren(std::move(children));
   return node;
 }
 
-std::unique_ptr<DOM::Node> DOMAgent::BuildDomNodeFromUIElement(
-    UIElement* root) {
-  std::unique_ptr<Array<DOM::Node>> children = Array<DOM::Node>::create();
+std::unique_ptr<Node> DOMAgent::BuildDomNodeFromUIElement(UIElement* root) {
+  std::unique_ptr<Array<Node>> children = Array<Node>::create();
   for (auto* it : root->children())
     children->addItem(BuildDomNodeFromUIElement(it));
 
@@ -135,19 +149,19 @@ std::unique_ptr<DOM::Node> DOMAgent::BuildDomNodeFromUIElement(
                    std::move(children), root->node_id());
 }
 
-std::unique_ptr<DOM::Node> DOMAgent::BuildInitialTree() {
-  is_building_tree_ = true;
-  std::unique_ptr<Array<DOM::Node>> children = Array<DOM::Node>::create();
+std::unique_ptr<Node> DOMAgent::BuildInitialTree() {
+  std::unique_ptr<Array<Node>> children = Array<Node>::create();
 
   element_root_ = std::make_unique<RootElement>(this);
+  element_root_->set_is_updating(true);
 
   for (auto* child : CreateChildrenForRoot()) {
     children->addItem(BuildTreeForUIElement(child));
     element_root_->AddChild(child);
   }
-  std::unique_ptr<DOM::Node> root_node =
+  std::unique_ptr<Node> root_node =
       BuildNode("root", nullptr, std::move(children), element_root_->node_id());
-  is_building_tree_ = false;
+  element_root_->set_is_updating(false);
   return root_node;
 }
 
@@ -164,7 +178,6 @@ void DOMAgent::RemoveDomNode(UIElement* ui_element) {
 }
 
 void DOMAgent::Reset() {
-  is_building_tree_ = false;
   element_root_.reset();
   node_id_to_ui_element_.clear();
   observers_.Clear();

components/ui_devtools/dom_agent.h

@@ -5,6 +5,11 @@
 #ifndef COMPONENTS_UI_DEVTOOLS_DOM_AGENT_H_
 #define COMPONENTS_UI_DEVTOOLS_DOM_AGENT_H_
 
+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
 #include "base/observer_list.h"
 #include "components/ui_devtools/DOM.h"
 #include "components/ui_devtools/devtools_base_agent.h"
@@ -44,7 +49,7 @@ class UI_DEVTOOLS_EXPORT DOMAgent
   void AddObserver(DOMAgentObserver* observer);
   void RemoveObserver(DOMAgentObserver* observer);
   UIElement* GetElementFromNodeId(int node_id) const;
-  UIElement* element_root() const { return element_root_.get(); };
+  UIElement* element_root() const { return element_root_.get(); }
 
   // Returns parent id of the element with id |node_id|. Returns 0 if parent
   // does not exist.
@@ -72,7 +77,6 @@ class UI_DEVTOOLS_EXPORT DOMAgent
   void RemoveDomNode(UIElement* ui_element);
   void Reset();
 
-  bool is_building_tree_ = false;
   std::unique_ptr<UIElement> element_root_;
   std::unordered_map<int, UIElement*> node_id_to_ui_element_;
 

components/ui_devtools/ui_element.h

@@ -6,6 +6,8 @@
 #define COMPONENTS_UI_DEVTOOLS_UI_ELEMENT_H_
 
 #include <memory>
+#include <string>
+#include <utility>
 #include <vector>
 
 #include "base/macros.h"
@@ -28,13 +30,15 @@ enum UIElementType { WINDOW, WIDGET, VIEW, ROOT, FRAMESINK, SURFACE };
 class UI_DEVTOOLS_EXPORT UIElement {
  public:
   virtual ~UIElement();
-  int node_id() const { return node_id_; };
+  int node_id() const { return node_id_; }
   std::string GetTypeName() const;
-  UIElement* parent() const { return parent_; };
-  void set_parent(UIElement* parent) { parent_ = parent; };
-  UIElementDelegate* delegate() const { return delegate_; };
-  UIElementType type() const { return type_; };
-  const std::vector<UIElement*>& children() const { return children_; };
+  UIElement* parent() const { return parent_; }
+  void set_parent(UIElement* parent) { parent_ = parent; }
+  UIElementDelegate* delegate() const { return delegate_; }
+  UIElementType type() const { return type_; }
+  const std::vector<UIElement*>& children() const { return children_; }
+  bool is_updating() const { return is_updating_; }
+  void set_is_updating(bool is_updating) { is_updating_ = is_updating; }
 
   // |child| is inserted in front of |before|. If |before| is null, it
   // is inserted at the end. Parent takes ownership of the added child.
@@ -70,7 +74,7 @@ class UI_DEVTOOLS_EXPORT UIElement {
   template <typename BackingT, typename T>
   static BackingT* GetBackingElement(const UIElement* element) {
     return T::From(element);
-  };
+  }
 
  protected:
   UIElement(const UIElementType type,
@@ -83,6 +87,7 @@ class UI_DEVTOOLS_EXPORT UIElement {
   std::vector<UIElement*> children_;
   UIElement* parent_;
   UIElementDelegate* delegate_;
+  bool is_updating_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(UIElement);
 };

components/ui_devtools/views/dom_agent_aura.cc

@@ -5,6 +5,8 @@
 #include "components/ui_devtools/views/dom_agent_aura.h"
 
 #include <memory>
+#include <utility>
+#include <vector>
 
 #include "components/ui_devtools/devtools_server.h"
 #include "components/ui_devtools/root_element.h"
@@ -22,7 +24,8 @@
 namespace ui_devtools {
 namespace {
 
-using namespace ui_devtools::protocol;
+using ui_devtools::protocol::DOM::Node;
+using ui_devtools::protocol::Array;
 // TODO(mhashmi): Make ids reusable
 
 views::Widget* GetWidgetFromWindow(gfx::NativeWindow window) {
@@ -52,7 +55,7 @@ std::vector<UIElement*> DOMAgentAura::CreateChildrenForRoot() {
   return children;
 }
 
-std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForUIElement(
+std::unique_ptr<Node> DOMAgentAura::BuildTreeForUIElement(
     UIElement* ui_element) {
   if (ui_element->type() == UIElementType::WINDOW) {
     return BuildTreeForWindow(
@@ -70,10 +73,10 @@ std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForUIElement(
   return nullptr;
 }
 
-std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForWindow(
+std::unique_ptr<Node> DOMAgentAura::BuildTreeForWindow(
     UIElement* window_element_root,
     aura::Window* window) {
-  std::unique_ptr<Array<DOM::Node>> children = Array<DOM::Node>::create();
+  std::unique_ptr<Array<Node>> children = Array<Node>::create();
   views::Widget* widget = GetWidgetFromWindow(window);
   if (widget) {
     UIElement* widget_element =
@@ -89,16 +92,16 @@ std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForWindow(
     children->addItem(BuildTreeForWindow(window_element, child));
     window_element_root->AddChild(window_element);
   }
-  std::unique_ptr<DOM::Node> node =
+  std::unique_ptr<Node> node =
       BuildNode("Window", window_element_root->GetAttributes(),
                 std::move(children), window_element_root->node_id());
   return node;
 }
 
-std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForRootWidget(
+std::unique_ptr<Node> DOMAgentAura::BuildTreeForRootWidget(
     UIElement* widget_element,
     views::Widget* widget) {
-  std::unique_ptr<Array<DOM::Node>> children = Array<DOM::Node>::create();
+  std::unique_ptr<Array<Node>> children = Array<Node>::create();
 
   UIElement* view_element =
       new ViewElement(widget->GetRootView(), this, widget_element);
@@ -106,24 +109,35 @@ std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForRootWidget(
   children->addItem(BuildTreeForView(view_element, widget->GetRootView()));
   widget_element->AddChild(view_element);
 
-  std::unique_ptr<DOM::Node> node =
+  std::unique_ptr<Node> node =
       BuildNode("Widget", widget_element->GetAttributes(), std::move(children),
                 widget_element->node_id());
   return node;
 }
 
-std::unique_ptr<DOM::Node> DOMAgentAura::BuildTreeForView(
-    UIElement* view_element,
-    views::View* view) {
-  std::unique_ptr<Array<DOM::Node>> children = Array<DOM::Node>::create();
+std::unique_ptr<Node> DOMAgentAura::BuildTreeForView(UIElement* view_element,
+                                                     views::View* view) {
+  std::unique_ptr<Array<Node>> children = Array<Node>::create();
 
   for (auto* child : view->GetChildrenInZOrder()) {
-    UIElement* view_element_child = new ViewElement(child, this, view_element);
+    // When building the subtree, a particular view could be visited multiple
+    // times because for each view of the subtree, we would call
+    // BuildTreeForView(..) on that view which causes the subtree with that view
+    // as root being visited again.  Here we check if we already constructed the
+    // ViewElement and skip true.
+    UIElement* view_element_child = nullptr;
+    auto id =
+        view_element->FindUIElementIdForBackendElement<views::View>(child);
+    if (id > 0) {
+      view_element_child = GetElementFromNodeId(id);
+    } else {
+      view_element_child = new ViewElement(child, this, view_element);
+      view_element->AddChild(view_element_child);
+    }
 
     children->addItem(BuildTreeForView(view_element_child, child));
-    view_element->AddChild(view_element_child);
   }
-  std::unique_ptr<DOM::Node> node =
+  std::unique_ptr<Node> node =
       BuildNode("View", view_element->GetAttributes(), std::move(children),
                 view_element->node_id());
   return node;