Snap RWHVA and resize the legacy window on Windows whenever the ancestor window's bounds change.

content/browser/renderer_host/render_widget_host_view_aura.cc

Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 146355707f08f247446929d5c6c0d22d7b5e11e2..df87be061118d8491325a268c19ee7cc49ac9dc7 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
 
+#include <set>
+
 #include "base/auto_reset.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ -358,6 +360,88 @@ class RenderWidgetHostViewAura::WindowObserver : public aura::WindowObserver {
   DISALLOW_COPY_AND_ASSIGN(WindowObserver);
 };
 
+// This class provides functionality to observe the ancestors of the RWHVA
+// window for bounds changes and snap the RWHVA instance to pixel boundaries.
+// Reason for doing this is there are cases like the fast resize code path for
+// bookmarks where in the parent of RWHVA which is WCV has its bounds changed
+// before the bookmark is hidden. This results in it reporting the old bounds
+// which includes the bookmark. Eventually when it does get the correct bounds
+// after the bar is hidden, the layer ignores the bounds changed call as the
+// bounds are the same. To work around this we observe the
+// ancestors of the RWHVA window for bounds changed notifications and snap when
+// we receive them.
+class RenderWidgetHostViewAura::WindowAncestorObserver
+    : public aura::WindowObserver {
+ public:
+  explicit WindowAncestorObserver(RenderWidgetHostViewAura* view)
+      : view_(view) {
+    view_->window_->AddObserver(this);
+  }
+
+  ~WindowAncestorObserver() override {
+    RemoveAncestorObservers();
+    view_->window_->RemoveObserver(this);
+  }
+
+  // Overridden from aura::WindowObserver:
+  void OnWindowHierarchyChanged(const HierarchyChangeParams& params) override {
+    if (!params.target->Contains(view_->window_) || !params.new_parent)

[sky, 2015/04/09 23:44:38]

I'm wondering if you could simplify things like the following:

. In WindowObserver override OnWindowHierarchyChanged() call through to RWHVA
and tell it the WindowHierarchyChanged.
. RWHVA::WindowHierarchyChanged would destroy the current WindowAncestorObserver
and create a new one.

My rationale is how you have things now you'll get OnWindowHierarchyChanged n!
times. Additionally what I've outlined is much simpler and less error prone. It
should also mean you don't need to worry about OnWindowDestroyed either, it's
handle by virtue of creating/destroying when the hierarchy changes.

[ananta, 2015/04/10 00:36:52]

Done. I added the hierarchy changed function in the current WindowObserver
class.

+      return;
+
+    if (params.old_parent)
+      StopObservingAncestor(params.old_parent);
+
+    // To ensure that we get notified for bounds changed notifications for all
+    // ancestors for the view window, we walk the ancestor hierarchy starting
+    // with the new parent and add observers for all ancestors we have not
+    // seen.
+    aura::Window* parent = params.new_parent;
+    while (parent) {
+      if (ancestor_observer_tracker_.find(parent) ==
+          ancestor_observer_tracker_.end()) {
+        parent->AddObserver(this);
+        ancestor_observer_tracker_.insert(parent);
+      }
+      parent = parent->parent();
+    }
+    view_->HandleParentBoundsChanged();
+  }
+
+  void OnWindowBoundsChanged(aura::Window* window,
+                             const gfx::Rect& old_bounds,
+                             const gfx::Rect& new_bounds) override {
+    if (ancestor_observer_tracker_.find(window) !=

[sky, 2015/04/09 23:44:38]

This should be a DCHECK.

[ananta, 2015/04/10 00:36:52]

Done.

+        ancestor_observer_tracker_.end()) {
+      view_->HandleParentBoundsChanged();

[sky, 2015/04/09 23:44:38]

Don't you only care about this if the origin changes? Size changes won't effect
the location on screen, right?

[ananta, 2015/04/10 00:36:52]

Yes. Added a check for origin change.

+    }
+  }
+
+  void OnWindowDestroyed(aura::Window* window) override {
+    StopObservingAncestor(window);
+  }
+
+ private:
+  void RemoveAncestorObservers() {
+    for (auto ancestor_observer_iterator : ancestor_observer_tracker_)

[sky, 2015/04/09 23:44:38]

ancestor_observer_iterator is a misleading name here. ancestor_observer_iterator
is really a window.

[ananta, 2015/04/10 00:36:52]

changed to ancestor

+      ancestor_observer_iterator->RemoveObserver(this);
+    ancestor_observer_tracker_.clear();
+  }
+
+  void StopObservingAncestor(aura::Window* ancestor) {
+    std::set<aura::Window*>::iterator index =

[sky, 2015/04/09 23:44:38]

index->iter

[ananta, 2015/04/10 00:36:52]

Function has been removed.

+        ancestor_observer_tracker_.find(ancestor);
+    if (index != ancestor_observer_tracker_.end()) {
+      (*index)->RemoveObserver(this);
+      ancestor_observer_tracker_.erase(index);
+    }
+  }
+
+  RenderWidgetHostViewAura* view_;
+  std::set<aura::Window*> ancestor_observer_tracker_;
+
+  DISALLOW_COPY_AND_ASSIGN(WindowAncestorObserver);
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewAura, public:
 
@@ -395,6 +479,8 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host,
     host_->SetView(this);
 
   window_observer_.reset(new WindowObserver(this));
+  ancestor_window_observer_.reset(new WindowAncestorObserver(this));
+
   aura::client::SetTooltipText(window_, &tooltip_);
   aura::client::SetActivationDelegate(window_, this);
   aura::client::SetFocusChangeObserver(window_, this);
@@ -734,6 +820,16 @@ bool RenderWidgetHostViewAura::CanRendererHandleEvent(
   return true;
 }
 
+void RenderWidgetHostViewAura::HandleParentBoundsChanged() {
+  SnapToPhysicalPixelBoundary();
+#if defined(OS_WIN)
+  if (legacy_render_widget_host_HWND_) {
+    legacy_render_widget_host_HWND_->SetBounds(
+        window_->GetBoundsInRootWindow());
+  }
+#endif
+}
+
 void RenderWidgetHostViewAura::MovePluginWindows(
     const std::vector<WebPluginGeometry>& plugin_window_moves) {
 #if defined(OS_WIN)