Refactors DimWindow and moves to ash/common

ash/common/wm/window_dimmer.h

 // Copyright 2015 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.
 
+#ifndef ASH_COMMON_WINDOW_DIMMER_H_
+#define ASH_COMMON_WINDOW_DIMMER_H_
+
 #include "ash/ash_export.h"
+#include "ash/common/wm_window_observer.h"
 #include "base/macros.h"
-#include "ui/aura/window.h"
-#include "ui/aura/window_observer.h"
 
 namespace ash {
 
-// A window used to dim the child windows of the given container.
-class ASH_EXPORT DimWindow : public aura::Window, public aura::WindowObserver {
+// WindowDimmer creates a window whose opacity is animated by way of
+// SetDimOpacity() and whose size matches that of its parent. WindowDimmer is
+// intended to be used in cases where a certain set of windows need to appear
+// partially obscured. This is achieved by creating WindowDimmer, setting the
+// opacity, and then stacking window() above the windows that are to appear
+// obscured.

[James Cook, 2016/09/09 21:33:36]

Maybe comment on the usual lifetime, specifically that it can be destroyed when
a controller like ScreenDimmer deletes it, or when the container window it is
blocking is destroyed? I know that's implied by storage in WmWindowUserData, but
I think it's unusual enough that it's worth a note.

[sky, 2016/09/09 21:50:45]

Done.

+class ASH_EXPORT WindowDimmer : public WmWindowObserver {

[sky, 2016/09/09 20:28:32]

I changed this code around so that it owns window_. Previously it didn't and
assumed window_ was owned by it's parent and deleted with it. That seems error
prone in the case of someone deleting WindowDimmer directly. I also made the
code deal with window_ being destroyed as well. That said, I made SetDimOpacity
DCHECK that window_ is valid. I went with that as I viewed the case of window_
being null as an error on the callers part, and not a case this code should deal
with.

[James Cook, 2016/09/09 21:33:36]

Acknowledged.

  public:
-  // Return a dim window for the container if any, or nullptr.
-  static DimWindow* Get(aura::Window* container);
-
-  explicit DimWindow(aura::Window* parent);
-  ~DimWindow() override;
+  // Creates a new WindowDimmer. The window() created by WindowDimmer is added
+  // to |parent| and stacked above all other child windows.
+  explicit WindowDimmer(WmWindow* parent);
+  ~WindowDimmer() override;
 
   void SetDimOpacity(float target_opacity);
 
-  // aura::WindowObserver:
-  void OnWindowBoundsChanged(aura::Window* window,
+  WmWindow* parent() { return parent_; }
+  WmWindow* window() { return window_; }
+
+  // NOTE: WindowDimmer is an observer for both |parent_| and |window_|.

[James Cook, 2016/09/09 21:33:36]

Thanks for documenting this.

+  // WmWindowObserver:
+  void OnWindowBoundsChanged(WmWindow* window,
                              const gfx::Rect& old_bounds,
                              const gfx::Rect& new_bounds) override;
-  void OnWindowDestroying(aura::Window* window) override;
+  void OnWindowDestroying(WmWindow* window) override;
+  void OnWindowTreeChanging(WmWindow* window,
+                            const TreeChangeParams& params) override;
 
  private:
-  aura::Window* parent_;
+  WmWindow* parent_;
+  WmWindow* window_;
 
-  DISALLOW_COPY_AND_ASSIGN(DimWindow);
+  DISALLOW_COPY_AND_ASSIGN(WindowDimmer);
 };
 
 }  // namespace ash
+
+#endif  // ASH_COMMON_WINDOW_DIMMER_H_