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

 // Copyright (c) 2012 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 "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"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/layers/layer.h"
@@ skipping 334 matching lines @@
     if (window == view_->window_)
       view_->RemovingFromRootWindow();
   }
 
  private:
   RenderWidgetHostViewAura* view_;
 
   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:
 
 RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host,
                                                    bool is_guest_view_hack)
     : host_(RenderWidgetHostImpl::From(host)),
       window_(new aura::Window(this)),
       delegated_frame_host_(new DelegatedFrameHost(this)),
       in_shutdown_(false),
       in_bounds_changed_(false),
@@ skipping 17 matching lines @@
       showing_context_menu_(false),
 #endif
       has_snapped_to_boundary_(false),
       touch_editing_client_(NULL),
       is_guest_view_hack_(is_guest_view_hack),
       weak_ptr_factory_(this) {
   if (!is_guest_view_hack_)
     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);
   window_->set_layer_owner_delegate(delegated_frame_host_.get());
   gfx::Screen::GetScreenFor(window_)->AddObserver(this);
 
   bool overscroll_enabled = base::CommandLine::ForCurrentProcess()->
       GetSwitchValueASCII(switches::kOverscrollHistoryNavigation) != "0";
   SetOverscrollControllerEnabled(overscroll_enabled);
 }
@@ skipping 319 matching lines @@
                                   ui::EF_RIGHT_MOUSE_BUTTON;
       return (event->flags() & kAllowedButtons) != 0;
     }
     default:
       break;
   }
 #endif
   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)
   // We need to clip the rectangle to the tab's viewport, otherwise we will draw
   // over the browser UI.
   if (!window_->GetRootWindow()) {
     DCHECK(plugin_window_moves.empty());
     return;
   }
   HWND parent = window_->GetHost()->GetAcceleratedWidget();
@@ skipping 1946 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewBase, public:
 
 // static
 void RenderWidgetHostViewBase::GetDefaultScreenInfo(WebScreenInfo* results) {
   GetScreenInfoForWindow(results, NULL);
 }
 
 }  // namespace content