Ash window cycle ui: don't create mirrored layers for previews until

ash/wm/window_mirror_view.cc

Index: ash/wm/window_mirror_view.cc
diff --git a/ash/wm/window_mirror_view.cc b/ash/wm/window_mirror_view.cc
index 1c50ecb5a2e7ed5a4c87b8bfb2eaf67644541ac6..00d426ffc50bc027369113e18db6fb303500486f 100644
--- a/ash/wm/window_mirror_view.cc
+++ b/ash/wm/window_mirror_view.cc
@@ -11,6 +11,7 @@
 #include "ui/aura/window.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_tree_owner.h"
+#include "ui/views/widget/widget.h"
 
 namespace ash {
 namespace wm {
@@ -34,27 +35,24 @@ WindowMirrorView::WindowMirrorView(WmWindowAura* window) : target_(window) {
 }
 WindowMirrorView::~WindowMirrorView() {}
 
-void WindowMirrorView::Init() {
-  SetPaintToLayer(true);
-
-  layer_owner_ = ::wm::RecreateLayers(target_->aura_window(), this);
-
-  GetMirrorLayer()->parent()->Remove(GetMirrorLayer());
-  layer()->Add(GetMirrorLayer());
-
-  // Some extra work is needed when the target window is minimized.
-  if (target_->GetWindowState()->IsMinimized()) {
-    GetMirrorLayer()->SetVisible(true);
-    GetMirrorLayer()->SetOpacity(1);
-    EnsureAllChildrenAreVisible(GetMirrorLayer());
-  }
-}
-
 gfx::Size WindowMirrorView::GetPreferredSize() const {
   return target_->GetBounds().size();
 }
 
 void WindowMirrorView::Layout() {
+  // Ensure the layer owner exists, but only if |this| is actually visible.
+  if (!layer_owner_) {
+    if (!GetVisibleBounds().IsEmpty()) {
+      InitLayerOwner();
+    } else {
+      // If |this| isn't on screen, no-op. We call InvalidateLayout() so
+      // Layout() will get called again next time even if the bounds haven't
+      // changed.
+      InvalidateLayout();

[sky, 2016/08/01 20:55:57]

Are you sure you want that and not overriding
GetNeedsNotificationWhenVisibleBoundsChange to return true?

[Evan Stade, 2016/08/01 22:11:14]

a) We don't really want notifications after the initial visible bounds change,
and it seems like using that approach would result in more code rather than
less. Then we'd need to also override OnVisibleBoundsChanged and init the layer
owner there and call Layout manually. Doesn't seem much better in my head but I
can try it if you prefer that path.

b) GetNeedsNotificationWhenVisibleBoundsChange says it's only used by
NativeViewHost, but it looks like that's not true, does the documentation need
updating?

[sky, 2016/08/02 16:21:27]

That's no different than the if (!layer_owner_) here, right?
I suggested using GetNeedsNotificationWhenVisibleBoundsChange because you want
to do something when the visible bounds change, and the InvalidateLayout() call
here invalidates the layout of all ancestors. That seems like overkill for what
you're trying to do. But what you have here certainly works, so if you feel
strongly I'm ok with it.

[Evan Stade, 2016/08/02 18:22:55]

OK, I'll try it the other way because it's easier to fairly evaluate when it's
actually in front of you, but I probably won't get to it till Friday so this
review may be dormant for a few days.

+      return;
+    }
+  }
+
   // Position at 0, 0.
   GetMirrorLayer()->SetBounds(gfx::Rect(GetMirrorLayer()->bounds().size()));
 
@@ -78,6 +76,22 @@ ui::LayerDelegate* WindowMirrorView::CreateDelegate(
   return delegates_.back().get();
 }
 
+void WindowMirrorView::InitLayerOwner() {
+  SetPaintToLayer(true);
+
+  layer_owner_ = ::wm::RecreateLayers(target_->aura_window(), this);
+
+  GetMirrorLayer()->parent()->Remove(GetMirrorLayer());
+  layer()->Add(GetMirrorLayer());
+
+  // Some extra work is needed when the target window is minimized.
+  if (target_->GetWindowState()->IsMinimized()) {
+    GetMirrorLayer()->SetVisible(true);
+    GetMirrorLayer()->SetOpacity(1);
+    EnsureAllChildrenAreVisible(GetMirrorLayer());
+  }
+}
+
 ui::Layer* WindowMirrorView::GetMirrorLayer() {
   return layer_owner_->root();
 }