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 <map>
+
 #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 307 matching lines @@
     // Note: popup_parent_host_view_ may be NULL when there are multiple
     // popup children per view. See: RenderWidgetHostViewAura::InitAsPopup().
     Shutdown();
   }
 }
 
 // We have to implement the WindowObserver interface on a separate object
 // because clang doesn't like implementing multiple interfaces that have
 // methods with the same name. This object is owned by the
 // RenderWidgetHostViewAura.
 class RenderWidgetHostViewAura::WindowObserver : public aura::WindowObserver {

[sky, 2015/04/09 00:01:14]

Using the same class for two very different purposes is error prone. I would
prefer another class that manages listening to all the ancestors.

And update the documentation for this class.

[ananta, 2015/04/09 03:36:05]

Done.

  public:
   explicit WindowObserver(RenderWidgetHostViewAura* view)
       : view_(view) {
     view_->window_->AddObserver(this);
   }
 
-  ~WindowObserver() override { view_->window_->RemoveObserver(this); }
+  ~WindowObserver() override {
+    RemoveAncestorObservers();
+    view_->window_->RemoveObserver(this);
+  }
 
   // Overridden from aura::WindowObserver:
   void OnWindowAddedToRootWindow(aura::Window* window) override {

[sky, 2015/04/09 00:01:14]

I don't think you're handling the case of ancestors changing correctly but the
root remains the same. I don't think you care
OnWindowAddedToRoot/RemovedFromRoot, but rather any time an ancestor is
added/removed. This is another reason to remove this logic into a standalone
class rather than here.

[ananta, 2015/04/09 03:36:05]

Done.

-    if (window == view_->window_)
+    if (window == view_->window_) {
+      // We observe the ancestors 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
+      // parent of WCV for bounds changed notifications and snap when we
+      // receive them.
+      aura::Window* parent = window->parent();
+      while (parent) {
+        parent->AddObserver(this);
+        ancestor_observer_map_[parent] = true;
+        parent = parent->parent();
+      }
       view_->AddedToRootWindow();
+    }
   }
 
   void OnWindowRemovingFromRootWindow(aura::Window* window,
                                       aura::Window* new_root) override {
-    if (window == view_->window_)
+    if (window == view_->window_) {
+      RemoveAncestorObservers();
       view_->RemovingFromRootWindow();
+    }
+  }
+
+  void OnWindowBoundsChanged(aura::Window* window,
+                             const gfx::Rect& old_bounds,
+                             const gfx::Rect& new_bounds) override {
+    if (ancestor_observer_map_.find(window) != ancestor_observer_map_.end())
+      view_->HandleParentBoundsChanged();
   }
 
  private:
+  void RemoveAncestorObservers() {
+    for (auto ancestor_iterator : ancestor_observer_map_)
+      ancestor_iterator.first->RemoveObserver(this);
+    ancestor_observer_map_.clear();
+  }
+
   RenderWidgetHostViewAura* view_;
+  std::map<aura::Window*, bool> ancestor_observer_map_;

[sky, 2015/04/09 00:01:14]

I don't think you need a map, how about a set?

[ananta, 2015/04/09 03:36:05]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(WindowObserver);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewAura, public:
 
 RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host,
                                                    bool is_guest_view_hack)
     : host_(RenderWidgetHostImpl::From(host)),
@@ skipping 360 matching lines @@
                                   ui::EF_RIGHT_MOUSE_BUTTON;
       return (event->flags() & kAllowedButtons) != 0;
     }
     default:
       break;
   }
 #endif
   return true;
 }
 
+void RenderWidgetHostViewAura::HandleParentBoundsChanged() {
+  SnapToPhysicalPixelBoundary();

[sky, 2015/04/09 00:01:14]

Does this mean we can remove some calls to SnapToPhysicalPixelBoundary.

[ananta, 2015/04/09 03:36:05]

Removed the call from WCVA.

+#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