Add ViewObserver to View for view updates

ui/views/devtools_observer.h

Index: ui/views/devtools_observer.h
diff --git a/ui/views/devtools_observer.h b/ui/views/devtools_observer.h
new file mode 100644
index 0000000000000000000000000000000000000000..08a5f8c055b8093bccb74fef978c7ef90bfc2ad2
--- /dev/null
+++ b/ui/views/devtools_observer.h
@@ -0,0 +1,43 @@
+// Copyright 2016 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.
+
+#ifndef UI_VIEWS_DEVTOOLS_OBSERVER_H_
+#define UI_VIEWS_DEVTOOLS_OBSERVER_H_
+
+#include "ui/views/view.h"
+#include "ui/views/views_export.h"
+
+namespace gfx {
+class Rect;
+}
+
+namespace views {
+
+// DevTools specific observer for View changes.
+class VIEWS_EXPORT DevToolsObserver {

[sky, 2016/11/19 00:46:31]

I don't think there is a good reason to limit this observer to just devtools.
Please rename to ViewsObserver with the typical add/remove pattern else where.

[Sarmad Hashmi, 2016/11/20 06:47:08]

Done.

[sadrul, 2016/11/21 18:07:33]

This was my suggestion to have a devtools-focused observer. My worry was that
there may be unwarranted use of this (especially since we have lived for so long
without a generic observer for View). But maybe that's not really true. I am OK
with a generic observer.

+ public:
+  // Called whenever target |view| is being moved. If |old_parent| is NULL,
+  // the target is being added. If the |new_parent| is NULL, then the target
+  // is being removed.
+  virtual void OnViewHierarchyChanged(View* target,

[sky, 2016/11/19 00:46:31]

This name has too much baggage with the existing View implementation. How about
OnViewParentChanged?

[Sarmad Hashmi, 2016/11/20 06:47:08]

Done.

+                                      View* old_parent,
+                                      View* new_parent) {}
+
+  virtual void OnViewVisibilityChanged(View* view, bool visible) {}

[sky, 2016/11/19 00:46:31]

Clearly indicate when this is sent. I assume it's just when the internal
visibility of the view changes, but the view may still be hidden if an ancestor
is hidden.

[Sarmad Hashmi, 2016/11/20 06:47:08]

Done.

+
+  virtual void OnViewEnabledChanged(View* view, int index) {}

[sky, 2016/11/19 00:46:31]

What is index here?

[Sarmad Hashmi, 2016/11/20 06:47:08]

Sorry, this was supposed to bool (enabled state). Removed it since
view->enabled() returns the same thing.

+
+  virtual void OnViewBoundsChanged(View* view, const gfx::Rect& new_bounds) {}

[sky, 2016/11/19 00:46:31]

Why do you need the new bounds? Can't the caller do view->bounds() if it wants
the new bounds? I could see passing the old.

[Sarmad Hashmi, 2016/11/20 06:47:08]

Fixed.

+
+  // Invoked whenever a child is moved to another index. This is called on the
+  // parent view. |view| is the child view being reordered to the index.
+  virtual void OnChildViewReordered(View* view, int index) {}
+
+ protected:
+  virtual ~DevToolsObserver() {}
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_DEVTOOLS_OBSERVER_H_