Show transient child when the transient parent is shown

ui/wm/core/transient_window_manager.cc

 // Copyright 2014 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 "ui/wm/core/transient_window_manager.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/auto_reset.h"
 #include "base/stl_util.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_property.h"
 #include "ui/wm/core/transient_window_observer.h"
 #include "ui/wm/core/transient_window_stacking_client.h"
 #include "ui/wm/core/window_util.h"
 
 using aura::Window;
 
 namespace wm {
+namespace {
 
 DEFINE_OWNED_WINDOW_PROPERTY_KEY(TransientWindowManager, kPropertyKey, NULL);
 
+}  // namespace
+
 TransientWindowManager::~TransientWindowManager() {
 }
 
 // static
 TransientWindowManager* TransientWindowManager::Get(Window* window) {
   TransientWindowManager* manager = window->GetProperty(kPropertyKey);
   if (!manager) {
     manager = new TransientWindowManager(window);
     window->SetProperty(kPropertyKey, manager);
   }
@@ skipping 56 matching lines @@
 }
 
 bool TransientWindowManager::IsStackingTransient(
     const aura::Window* target) const {
   return stacking_target_ == target;
 }
 
 TransientWindowManager::TransientWindowManager(Window* window)
     : window_(window),
       transient_parent_(NULL),
-      stacking_target_(NULL) {
+      stacking_target_(NULL),
+      parent_control_visibility_(false),
+      show_on_parent_visible_(false),
+      ignore_visibility_changed_event_(false) {
   window_->AddObserver(this);
 }
 
 void TransientWindowManager::RestackTransientDescendants() {
   Window* parent = window_->parent();
   if (!parent)
     return;
 
   // Stack any transient children that share the same parent to be in front of
   // |window_|. The existing stacking order is preserved by iterating backwards
@@ skipping 16 matching lines @@
   DCHECK_EQ(window_, window);
   // Stack |window| properly if it is transient child of a sibling.
   Window* transient_parent = wm::GetTransientParent(window);
   if (transient_parent && transient_parent->parent() == parent) {
     TransientWindowManager* transient_parent_manager =
         Get(transient_parent);
     transient_parent_manager->RestackTransientDescendants();
   }
 }
 
+void TransientWindowManager::UpdateTransientChildVisibility(
+    bool parent_visible) {
+  base::AutoReset<bool> reset(&ignore_visibility_changed_event_, true);
+  if (!parent_visible) {
+    show_on_parent_visible_ = window_->TargetVisibility();
+    window_->Hide();
+  } else {
+    if (show_on_parent_visible_ && parent_control_visibility_)
+      window_->Show();
+    show_on_parent_visible_ = false;
+  }
+}
+
 void TransientWindowManager::OnWindowVisibilityChanging(Window* window,
                                                         bool visible) {
-  // TODO(sky): move handling of becoming visible here.
-  if (!visible) {
-    std::for_each(transient_children_.begin(), transient_children_.end(),
-                  std::mem_fun(&Window::Hide));
+  if (window_ != window)

[sky, 2014/10/07 15:19:14]

Why the if, at best this should be a DCHECK

[oshima, 2014/10/07 17:41:14]

Done. I was confused by the difference between Changing (called on the target
window only) and Changing (called in hierarchy chain)

[oshima, 2014/10/07 17:41:56]

oops, I meant "Changed (called in hierarchy chain)"

+    return;
+
+  for (auto* child : transient_children_)

[sky, 2014/10/07 15:19:14]

From the style guide: "auto is permitted, for local variables only. Do not use
auto for file-scope or namespace-scope variables, or for class members."

[oshima, 2014/10/07 17:41:14]

Where is it? I was looking at

http://chromium-cpp.appspot.com/

and couldn't find that description.

[oshima, 2014/10/07 17:46:25]

I guess you meant the description in
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#auto 

I believe what this is prohibiting is something like

auto global = 1;

namespace {
auto namespace_scope = "xyz";
}

The auto in the loop creates local variable, so this should be fine.

[sky, 2014/10/07 19:34:21]

Yes, you are right. My mistake.

+    Get(child)->UpdateTransientChildVisibility(visible);
+}
+
+void TransientWindowManager::OnWindowVisibilityChanged(Window* window,

[sky, 2014/10/07 15:19:14]

Why do you need to do something in both changed and changing? Isn't one enough?

[oshima, 2014/10/07 17:41:14]

I tried and had problem because OnWindowVisibiltyChanged is called twice on the
same target (during Down and Up traversal). Is this by design?

[sky, 2014/10/07 19:34:21]

That seems wrong. It should only be called once.

+                                                       bool visible) {
+  if (window_ != window || ignore_visibility_changed_event_ ||

[sky, 2014/10/07 15:19:14]

You shouldn't need the window_ != window. Add a DCHECK if you want it.

[oshima, 2014/10/07 17:41:14]

I needed this because OnWindowVisibilityChanged is called on hierarchy chain

[sky, 2014/10/07 19:34:21]

Ah, ok.

+      !transient_parent_ || !parent_control_visibility_) {
+    return;
+  }
+  if (!transient_parent_->TargetVisibility() && visible) {
+    base::AutoReset<bool> reset(&ignore_visibility_changed_event_, true);
+    show_on_parent_visible_ = true;
+    window_->Hide();
+  } else if (!visible) {
+    DCHECK(!show_on_parent_visible_);
   }
 }
 
 void TransientWindowManager::OnWindowStackingChanged(Window* window) {
   DCHECK_EQ(window_, window);
-
   // Do nothing if we initiated the stacking change.
   const TransientWindowManager* transient_manager =
       Get(static_cast<const Window*>(window));
   if (transient_manager && transient_manager->stacking_target_) {
     Windows::const_iterator window_i = std::find(
         window->parent()->children().begin(),
         window->parent()->children().end(),
         window);
     DCHECK(window_i != window->parent()->children().end());
     if (window_i != window->parent()->children().begin() &&
@@ skipping 14 matching lines @@
 
   // Destroy transient children, only after we've removed ourselves from our
   // parent, as destroying an active transient child may otherwise attempt to
   // refocus us.
   Windows transient_children(transient_children_);
   STLDeleteElements(&transient_children);
   DCHECK(transient_children_.empty());
 }
 
 }  // namespace wm