Make code path for bounds changes getting to renderer less brittle

content/browser/web_contents/web_contents_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/web_contents/web_contents_view_aura.h"
 
+#include <set>
+
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/dip_util.h"
 #include "content/browser/renderer_host/overscroll_controller.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
@@ skipping 624 matching lines @@
   ImageLayerDelegate layer_delegate_;
 
   // During tests, the aura windows don't get any paint updates. So the overlay
   // container keeps waiting for a paint update it never receives, causing a
   // timeout. So during tests, disable the wait for paint updates.
   bool need_paint_update_;
 
   DISALLOW_COPY_AND_ASSIGN(OverscrollNavigationOverlay);
 };
 
 class WebContentsViewAura::WindowObserver

[sky, 2013/11/04 21:33:23]

Can you move this into its own standalone object so that you can easily write a
test?

[rharrison, 2013/11/06 19:26:17]

Done.

     : public aura::WindowObserver, public aura::RootWindowObserver {
  public:
   explicit WindowObserver(WebContentsViewAura* view)
-      : view_(view),
-        parent_(NULL) {
+      : view_(view) {
     view_->window_->AddObserver(this);
+    std::set<aura::Window*> ancestors = GenerateAncestors(view_->window_.get());
+    AddToObserved(ancestors);
   }
 
   virtual ~WindowObserver() {
     view_->window_->RemoveObserver(this);
     if (view_->window_->GetDispatcher())
       view_->window_->GetDispatcher()->RemoveRootWindowObserver(this);
-    if (parent_)
-      parent_->RemoveObserver(this);
+    RemoveFromObserved(observed_);
   }
 
   // Overridden from aura::WindowObserver:
   virtual void OnWindowParentChanged(aura::Window* window,
                                      aura::Window* parent) OVERRIDE {
-    if (window == parent_)
-      return;
-    if (parent_)
-      parent_->RemoveObserver(this);
-    parent_ = parent;
-    if (parent)
-      parent->AddObserver(this);
+    std::set<aura::Window*> ancestors = GenerateAncestors(view_->window_.get());
+    RemoveFromObserved(RelativeComplement(ancestors, observed_));
+    AddToObserved(RelativeComplement(observed_, ancestors));
   }
 
   virtual void OnWindowBoundsChanged(aura::Window* window,
                                      const gfx::Rect& old_bounds,
                                      const gfx::Rect& new_bounds) OVERRIDE {
     SendScreenRects();
     if (view_->touch_editable_)
       view_->touch_editable_->UpdateEditingController();

[sky, 2013/11/04 21:33:23]

Does this really need to be done any time the bounds of an ancestor changes?

[rharrison, 2013/11/06 19:26:17]

Talked to varunjain@ about this and I think it does. It has the same basic
requirement as sending screen rects, it needs to occur whenever the visible
bounds may have changed.

   }
 
   virtual void OnWindowAddedToRootWindow(aura::Window* window) OVERRIDE {
-    if (window != parent_)
+    if (window == view_->window_.get())
       window->GetDispatcher()->AddRootWindowObserver(this);
   }
 
   virtual void OnWindowRemovingFromRootWindow(aura::Window* window) OVERRIDE {
-    if (window != parent_)
+    if (window == view_->window_.get())
       window->GetDispatcher()->RemoveRootWindowObserver(this);
   }
 
   // Overridden RootWindowObserver:
   virtual void OnRootWindowHostMoved(const aura::RootWindow* root,
                                      const gfx::Point& new_origin) OVERRIDE {
     // This is for the desktop case (i.e. Aura desktop).
     SendScreenRects();
   }
 
  private:
   void SendScreenRects() {
     RenderWidgetHostImpl::From(view_->web_contents_->GetRenderViewHost())->
         SendScreenRects();
   }
 
+  std::set<aura::Window*> GenerateAncestors(aura::Window* window) {
+    std::set<aura::Window*> ret_val;
+    while (window->parent()) {
+      ret_val.insert(window->parent());
+      window = window->parent();
+    }
+    return ret_val;
+  }
+
+  void AddToObserved(std::set<aura::Window*> windows) {

[sky, 2013/11/04 21:34:01]

const std::set&

[rharrison, 2013/11/06 19:26:17]

Done.

+    for (std::set<aura::Window*>::iterator iter = windows.begin();
+         iter != windows.end();
+         ++iter) {
+      if (observed_.end() == observed_.find(*iter)) {
+        (*iter)->AddObserver(this);
+        observed_.insert(*iter);
+      }
+    }
+  }
+
+  void RemoveFromObserved(std::set<aura::Window*> windows) {

[sky, 2013/11/04 21:34:01]

const std::set&

[rharrison, 2013/11/06 19:26:17]

Done.

+    for (std::set<aura::Window*>::iterator iter = windows.begin();
+         iter != windows.end();
+         ++iter) {
+      if (observed_.end() != observed_.find(*iter)) {
+        (*iter)->RemoveObserver(this);
+        observed_.erase(*iter);
+      }
+    }
+  }
+
+  // Calculates the set of items in |set_b|, but not in |set_a|.
+  std::set<aura::Window*> RelativeComplement(std::set<aura::Window*> set_a,

[sky, 2013/11/04 21:33:23]

Does std offer some function that does this?

[sky, 2013/11/04 21:34:01]

const std::set&

[rharrison, 2013/11/06 19:26:17]

Apparently it is std::set_difference... I probably would not have guessed that
on my own.

+                                             std::set<aura::Window*> set_b) {
+    std::set<aura::Window*> ret_val;
+    for (std::set<aura::Window*>::iterator iter = set_b.begin();
+         iter != set_b.end();
+         ++iter) {
+      if (set_a.end() == set_a.find(*iter))
+        ret_val.insert(*iter);
+    }
+    return ret_val;
+  }
+
   WebContentsViewAura* view_;
-
-  // We cache the old parent so that we can unregister when it's not the parent
-  // anymore.
-  aura::Window* parent_;
+  std::set<aura::Window*> observed_;
 
   DISALLOW_COPY_AND_ASSIGN(WindowObserver);
 };
 
 #if defined(OS_WIN)
 // Constrained windows are added as children of the WebContent's view which may
 // overlap with windowed NPAPI plugins. In that case, tell the RWHV so that it
 // can update the plugins' cutout rects accordingly.
 class WebContentsViewAura::ChildWindowObserver : public aura::WindowObserver,
                                                  public WebContentsObserver {
@@ skipping 920 matching lines @@
       event.location(),
       gfx::Screen::GetScreenFor(GetNativeView())->GetCursorScreenPoint(),
       ConvertAuraEventFlagsToWebInputEventModifiers(event.flags()));
   if (drag_dest_delegate_)
     drag_dest_delegate_->OnDrop();
   current_drop_data_.reset();
   return current_drag_op_;
 }
 
 }  // namespace content