Create a unified UIElement interface for Widget, View and Window.

ash/devtools/view_element.cc

Index: ash/devtools/view_element.cc
diff --git a/ash/devtools/view_element.cc b/ash/devtools/view_element.cc
new file mode 100644
index 0000000000000000000000000000000000000000..50a34534b66a5e8a73c073a5a7f7fc3bac1fa3e6
--- /dev/null
+++ b/ash/devtools/view_element.cc
@@ -0,0 +1,103 @@
+// 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 "ash/devtools/view_element.h"
+
+#include "ash/devtools/ui_element_delegate.h"
+#include "ui/views/widget/widget.h"
+
+namespace ash {
+namespace devtools {
+namespace {
+
+int FindSibling(views::View* view) {

[sadrul, 2017/05/09 04:58:07]

Call this FindIndexInParent() or something like that.

[thanhph, 2017/05/09 20:52:46]

Done.

+  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_GE(view_index, 0);

[sadrul, 2017/05/09 04:58:07]

Use View::GetIndexOf() instead.

[thanhph, 2017/05/09 20:52:47]

cool, I removed FindSibling().

+  return view_index;
+}
+
+}  // namespace
+
+ViewElement::ViewElement(views::View* view,
+                         UIElementDelegate* ui_element_delegate,
+                         UIElement* parent)
+    : UIElement(UIElementType::VIEW, ui_element_delegate, parent), view_(view) {
+  view_->AddObserver(this);
+}
+
+ViewElement::~ViewElement() {
+  view_->RemoveObserver(this);
+}
+
+void ViewElement::OnChildViewRemoved(views::View* parent, views::View* view) {
+  DCHECK(parent == view_);
+  for (auto* child_element : children()) {
+    if (UIElement::GetBackingElement<views::View, ViewElement>(child_element) ==
+            view &&
+        delegate()->OnUIElementRemoved(child_element)) {
+      RemoveChild(child_element);
+      child_element->Destroy();
+    }
+  }

[sadrul, 2017/05/09 04:58:07]

You can avoid the for-loop here:

  auto iter = std::find_if(children().begin(), children().end(),
     [view](UIElement* child) {
       return view == UIElement::GetBackingElement<...>(child);
     });
  DCHECK(iter != children().end());
  UIElement* child_element = *iter;
  if (!delegate()->OnUIElementRemoved(child_element))
    return;
  RemoveChild(child_element);
  child_element->Destroy();

[thanhph, 2017/05/09 20:52:47]

Nice, done!

+}
+
+void ViewElement::OnChildViewAdded(views::View* parent, views::View* view) {
+  DCHECK(parent == view_);

[sadrul, 2017/05/09 04:58:07]

DCHECK_EQ (here and elsewhere)

[thanhph, 2017/05/09 20:52:47]

Done.

+  AddChild(new ViewElement(view, delegate(), this),
+           children().empty() ? nullptr : children().back());
+}
+
+void ViewElement::OnChildViewReordered(views::View* parent, views::View* view) {
+  DCHECK(view == view_);

[sadrul, 2017/05/09 04:58:07]

This is wrong, right? Because according to the comment (and code) [1], |view_|
would be |parent| (i.e. the parent is the observed view). The following code,
therefore, is also incorrect.

[1]
https://cs.chromium.org/chromium/src/ui/views/view_observer.h?type=cs&sq=pack...

[thanhph, 2017/05/09 20:52:47]

Good catch, unittests havent covered this so I added a case in
AshDevtoolsTest.ViewRearranged which hits here.

+  this->parent()->RemoveChild(this);
+  this->parent()->ReorderChild(this, FindSibling(view));
+  delegate()->OnUIElementReordered(this->parent(), this);
+}
+
+void ViewElement::OnViewBoundsChanged(views::View* view) {
+  DCHECK(view_ == view);

[sadrul, 2017/05/09 04:58:07]

Use DCHECK_EQ

[thanhph, 2017/05/09 20:52:46]

Done.

+  delegate()->OnUIElementBoundsChanged(this);
+}
+
+void ViewElement::Destroy() {
+  delete this;
+}
+
+void ViewElement::GetBounds(gfx::Rect* bounds) const {
+  *bounds = view_->bounds();
+}
+
+void ViewElement::SetBounds(const gfx::Rect& bounds) {
+  view_->SetBoundsRect(bounds);
+}
+
+void ViewElement::GetVisible(bool* visible) const {
+  *visible = view_->visible();
+}
+
+void ViewElement::SetVisible(bool visible) {
+  if (visible != view_->visible())
+    view_->SetVisible(visible);
+}
+
+std::pair<aura::Window*, gfx::Rect> ViewElement::GetNodeWindowAndBounds()
+    const {
+  return std::make_pair(view_->GetWidget()->GetNativeWindow(), view_->bounds());
+}
+
+// static
+views::View* ViewElement::From(UIElement* element) {
+  DCHECK_EQ(UIElementType::VIEW, element->type());
+  return static_cast<ViewElement*>(element)->view_;
+}
+
+}  // namespace devtools
+}  // namespace ash