views: Some more layer change, and add/update some tests for layers.

Hide the RootView's layer when a Widget is hidden. Call OnNativeWidgetVisibilityChanged on NativeWidgetGtk's hide/show callbacks.

The tests:
. Make sure Layer bounds change with a View even if it's not visible.
. Make sure Layer visibility is updated with a View's visibility.
. Make sure Layers get unparented properly when a View is removed from the parent view.

BUG=none
TEST=views_unittests

Review URL: http://codereview.chromium.org/8202026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@104846 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 82 insertions, 27 deletions

diff --git a/views/aura_desktop/aura_desktop_main.cc b/views/aura_desktop/aura_desktop_main.cc
index f600b31..aca5b14 100644
--- a/views/aura_desktop/aura_desktop_main.cc
+++ b/views/aura_desktop/aura_desktop_main.cc
@@ -62,6 +62,8 @@ class DemoWindowDelegate : public aura::WindowDelegate {
   }
   virtual void OnWindowDestroyed() OVERRIDE {
   }
+  virtual void OnWindowVisibilityChanged(bool visible) OVERRIDE {
+  }
 
  private:
   SkColor color_;
diff --git a/views/controls/native_control_gtk.cc b/views/controls/native_control_gtk.cc
index 965d347..a798bc6 100644
--- a/views/controls/native_control_gtk.cc
+++ b/views/controls/native_control_gtk.cc
@@ -172,15 +172,7 @@ void NativeControlGtk::ViewHierarchyChanged(bool is_add, View* parent,
 }
 
 void NativeControlGtk::VisibilityChanged(View* starting_from, bool is_visible) {
-  if (!is_visible) {
-    if (native_view()) {
-      // We destroy the child widget when we become invisible because of the
-      // performance cost of maintaining widgets that aren't currently needed.
-      Detach();
-      // Make sure that Detach destroyed the widget.
-      DCHECK(!native_view());
-    }
-  } else if (!native_view()) {
+  if (!native_view()) {
     if (GetWidget())
       CreateNativeControl();
   } else {
diff --git a/views/view_unittest.cc b/views/view_unittest.cc
index c6bfab2..2015658 100644
--- a/views/view_unittest.cc
+++ b/views/view_unittest.cc
@@ -45,6 +45,17 @@
 
 using ::testing::_;
 
+namespace {
+
+// Returns true if |ancestor| is an ancestor of |layer|.
+bool LayerIsAncestor(const ui::Layer* ancestor, const ui::Layer* layer) {
+  while (layer && layer != ancestor)
+    layer = layer->parent();
+  return layer == ancestor;
+}
+
+}
+
 namespace views {
 
 typedef ViewsTestBase ViewTest;
@@ -2566,6 +2577,15 @@ TEST_F(ViewLayerTest, BoundsChangeWithLayer) {
 
   v2->SetPosition(gfx::Point(11, 12));
   EXPECT_EQ(gfx::Rect(36, 48, 40, 50), v2->layer()->bounds());
+
+  // Bounds of the layer should change even if the view is not invisible.
+  v1->SetVisible(false);
+  v1->SetPosition(gfx::Point(20, 30));
+  EXPECT_EQ(gfx::Rect(31, 42, 40, 50), v2->layer()->bounds());
+
+  v2->SetVisible(false);
+  v2->SetBounds(10, 11, 20, 30);
+  EXPECT_EQ(gfx::Rect(30, 41, 20, 30), v2->layer()->bounds());
 }
 
 // Makes sure a transform persists after toggling the visibility.
@@ -2604,23 +2624,38 @@ TEST_F(ViewLayerTest, ResetTransformOnLayerAfterAdd) {
   EXPECT_EQ(2.0f, view->layer()->transform().matrix().get(0, 0));
 }
 
-// Makes sure that layer persists after toggling the visibility.
+// Makes sure that layer visibility is correct after toggling View visibility.
 TEST_F(ViewLayerTest, ToggleVisibilityWithLayer) {
   View* content_view = new View;
   widget()->SetContentsView(content_view);
 
+  // The view isn't attached to a widget or a parent view yet. But it should
+  // still have a layer, but the layer should not be attached to the root
+  // layer.
   View* v1 = new View;
-  content_view->AddChildView(v1);
   v1->SetPaintToLayer(true);
-
-  v1->SetVisible(true);
   EXPECT_TRUE(v1->layer());
+  EXPECT_FALSE(LayerIsAncestor(widget()->GetCompositor()->root_layer(),
+                               v1->layer()));
+
+  // Once the view is attached to a widget, its layer should be attached to the
+  // root layer and visible.
+  content_view->AddChildView(v1);
+  EXPECT_TRUE(LayerIsAncestor(widget()->GetCompositor()->root_layer(),
+                              v1->layer()));
+  EXPECT_TRUE(v1->layer()->IsDrawn());
 
   v1->SetVisible(false);
-  EXPECT_TRUE(v1->layer());
+  EXPECT_FALSE(v1->layer()->IsDrawn());
 
   v1->SetVisible(true);
-  EXPECT_TRUE(v1->layer());
+  EXPECT_TRUE(v1->layer()->IsDrawn());
+
+  widget()->Hide();
+  EXPECT_FALSE(v1->layer()->IsDrawn());
+
+  widget()->Show();
+  EXPECT_TRUE(v1->layer()->IsDrawn());
 }
 
 // Test that a hole in a layer is correctly created regardless of whether
@@ -2717,6 +2752,33 @@ TEST_F(ViewLayerTest, ToggleVisibilityWithOpaqueLayer) {
       gfx::Rect(50, 50, 100, 100), parent_view->layer()->hole_rect());
 }
 
+// Tests that the layers in the subtree are orphaned after a View is removed
+// from the parent.
+TEST_F(ViewLayerTest, OrphanLayerAfterViewRemove) {
+  View* content_view = new View;
+  widget()->SetContentsView(content_view);
+
+  View* v1 = new View;
+  content_view->AddChildView(v1);
+
+  View* v2 = new View;
+  v1->AddChildView(v2);
+  v2->SetPaintToLayer(true);
+  EXPECT_TRUE(LayerIsAncestor(widget()->GetCompositor()->root_layer(),
+                              v2->layer()));
+  EXPECT_TRUE(v2->layer()->IsDrawn());
+
+  content_view->RemoveChildView(v1);
+  EXPECT_FALSE(LayerIsAncestor(widget()->GetCompositor()->root_layer(),
+                               v2->layer()));
+
+  // Reparent |v2|.
+  content_view->AddChildView(v2);
+  EXPECT_TRUE(LayerIsAncestor(widget()->GetCompositor()->root_layer(),
+                              v2->layer()));
+  EXPECT_TRUE(v2->layer()->IsDrawn());
+}
+
 // TODO(sky): reenable once focus issues are straightened out so that this
 // doesn't crash.
 TEST_F(ViewLayerTest, DISABLED_NativeWidgetView) {
diff --git a/views/widget/native_widget_aura.cc b/views/widget/native_widget_aura.cc
index 1060f40..d910843 100644
--- a/views/widget/native_widget_aura.cc
+++ b/views/widget/native_widget_aura.cc
@@ -532,6 +532,10 @@ void NativeWidgetAura::OnWindowDestroyed() {
     delete this;
 }
 
+void NativeWidgetAura::OnWindowVisibilityChanged(bool visible) {
+  delegate_->OnNativeWidgetVisibilityChanged(visible);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Widget, public:
 
diff --git a/views/widget/native_widget_aura.h b/views/widget/native_widget_aura.h
index 70cd7ae..23bb405 100644
--- a/views/widget/native_widget_aura.h
+++ b/views/widget/native_widget_aura.h
@@ -135,6 +135,7 @@ class VIEWS_EXPORT NativeWidgetAura : public internal::NativeWidgetPrivate,
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
   virtual void OnWindowDestroying() OVERRIDE;
   virtual void OnWindowDestroyed() OVERRIDE;
+  virtual void OnWindowVisibilityChanged(bool visible) OVERRIDE;
 
  private:
   typedef std::map<const char*, void*> PropsMap;
diff --git a/views/widget/native_widget_gtk.cc b/views/widget/native_widget_gtk.cc
index a160342..8f9b735 100644
--- a/views/widget/native_widget_gtk.cc
+++ b/views/widget/native_widget_gtk.cc
@@ -1196,8 +1196,6 @@ void NativeWidgetGtk::Show() {
     gtk_widget_show(widget_);
     if (widget_->window)
       gdk_window_raise(widget_->window);
-    // See Hide() for the reason why we're not calling
-    // OnNativeWidgetVisibilityChange.
   }
 }
 
@@ -1206,15 +1204,6 @@ void NativeWidgetGtk::Hide() {
     gtk_widget_hide(widget_);
     if (widget_->window)
       gdk_window_lower(widget_->window);
-    // We're not calling OnNativeWidgetVisibilityChanged because it
-    // breaks the ability to refocus to FindBar. NativeControlGtk
-    // detaches the underlying gtk widget for optimization purpose
-    // when it becomes invisible, which in turn breaks SetNativeFocus
-    // because there is no gtk attached to NativeControlGtk. I'm not
-    // fixing that part because
-    // a) This is views/gtk only issue, which will be gone soon.
-    // b) Alternative fix, which we can modify animator to show it
-    // immediately, won't be necessary for non gtk implementation.
   }
 }
 
@@ -1825,6 +1814,7 @@ void NativeWidgetGtk::OnDestroyed(GObject *where_the_object_was) {
 }
 
 void NativeWidgetGtk::OnShow(GtkWidget* widget) {
+  delegate_->OnNativeWidgetVisibilityChanged(true);
 }
 
 void NativeWidgetGtk::OnMap(GtkWidget* widget) {
@@ -1841,6 +1831,7 @@ void NativeWidgetGtk::OnMap(GtkWidget* widget) {
 }
 
 void NativeWidgetGtk::OnHide(GtkWidget* widget) {
+  delegate_->OnNativeWidgetVisibilityChanged(false);
 }
 
 gboolean NativeWidgetGtk::OnWindowStateEvent(GtkWidget* widget,
diff --git a/views/widget/widget.cc b/views/widget/widget.cc
index 9ce902a..8baf11c 100644
--- a/views/widget/widget.cc
+++ b/views/widget/widget.cc
@@ -870,9 +870,12 @@ void Widget::OnNativeBlur(gfx::NativeView focused_view) {
 }
 
 void Widget::OnNativeWidgetVisibilityChanged(bool visible) {
-  GetRootView()->PropagateVisibilityNotifications(GetRootView(), visible);
+  View* root = GetRootView();
+  root->PropagateVisibilityNotifications(root, visible);
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnWidgetVisibilityChanged(this, visible));
+  if (GetCompositor() && root->layer())
+    root->layer()->SetVisible(visible);
 }
 
 void Widget::OnNativeWidgetCreated() {