Skip to content

Commit

Permalink
Make comments more accurate in aura::WindowObserver.
Browse files Browse the repository at this point in the history
OnWindowBoundsChanged() is called when the bounds of a window change.

OnWindowOpacityChanged() and OnWindowTransformed() are called when
the opacity or transform of a window is set, even if it didn't change.

The reason for this difference is that the opacity and transform can
be animated on the impl side of cc. cc sets the property value on the
ui::Layer before the first frame of the animation is rendered and
when the animation ends, but not during the animation. Most of the
time, the value set before the first frame of the animation is
rendered is the same as the existing value. We want to notify
WindowObservers even if the value didn't change yet so that they
are aware that an animation started.

This CL also renamed OnWindowOpacityChanged -> OnWindowOpacitySet.

Bug: 738387
Change-Id: I75c7c449ae5d20ae5b76e3ca0b6bb041ce8b1439
Reviewed-on: https://chromium-review.googlesource.com/775822
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519427}
  • Loading branch information
fdoray authored and Commit Bot committed Nov 27, 2017
1 parent 50d262e commit 30b02ac
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 25 deletions.
6 changes: 3 additions & 3 deletions ui/aura/window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ void Window::OnLayerBoundsChanged(const gfx::Rect& old_bounds,
void Window::OnLayerOpacityChanged(ui::PropertyChangeReason reason) {
WindowOcclusionTracker::ScopedPauseOcclusionTracking pause_occlusion_tracking;
for (WindowObserver& observer : observers_)
observer.OnWindowOpacityChanged(this, reason);
observer.OnWindowOpacitySet(this, reason);
}

void Window::OnLayerTransformed(ui::PropertyChangeReason reason) {
Expand Down Expand Up @@ -1180,8 +1180,8 @@ std::unique_ptr<ui::Layer> Window::RecreateLayer() {
// animation ends.
if (was_animating_opacity) {
for (WindowObserver& observer : observers_) {
observer.OnWindowOpacityChanged(this,
ui::PropertyChangeReason::FROM_ANIMATION);
observer.OnWindowOpacitySet(this,
ui::PropertyChangeReason::FROM_ANIMATION);
}
}
if (was_animating_transform) {
Expand Down
30 changes: 16 additions & 14 deletions ui/aura/window_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class AURA_EXPORT WindowObserver {
// on.
virtual void OnWindowVisibilityChanged(Window* window, bool visible) {}

// Invoked when the bounds of the |window|'s layer are set. |old_bounds| and
// Invoked when the bounds of the |window|'s layer change. |old_bounds| and
// |new_bounds| are in parent coordinates. |reason| indicates whether the
// bounds were set directly or by an animation. This will be called at every
// step of a bounds animation. The client can determine whether the animation
Expand All @@ -91,26 +91,28 @@ class AURA_EXPORT WindowObserver {
const gfx::Rect& new_bounds,
ui::PropertyChangeReason reason) {}

// Invoked when the opacity of the |window|'s layer is set. |reason| indicates
// whether the opacity was set directly or by an animation. This won't
// necessarily be called at every step of an animation. However, it will
// always be called before the first frame of the animation is rendered and
// when the animation ends. The client can determine whether the animation is
// ending by calling window->layer()->GetAnimator()->IsAnimatingProperty(
// Invoked when the opacity of the |window|'s layer is set (even if it didn't
// change). |reason| indicates whether the opacity was set directly or by an
// animation. This won't necessarily be called at every step of an animation.
// However, it will always be called before the first frame of the animation
// is rendered and when the animation ends. The client can determine whether
// the animation is ending by calling
// window->layer()->GetAnimator()->IsAnimatingProperty(
// ui::LayerAnimationElement::OPACITY).
virtual void OnWindowOpacityChanged(Window* window,
ui::PropertyChangeReason reason) {}
virtual void OnWindowOpacitySet(Window* window,
ui::PropertyChangeReason reason) {}

// Invoked before Window::SetTransform() sets the transform of a window.
virtual void OnWindowTargetTransformChanging(
Window* window,
const gfx::Transform& new_transform) {}

// Invoked when the transform of |window| changes. |reason| indicates whether
// the transform was set directly or by an animation. This won't necessarily
// be called at every step of an animation. However, it will always be called
// before the first frame of the animation is rendered and when the animation
// ends. The client can determine whether the animation is ending by calling
// Invoked when the transform of |window| is set (even if it didn't change).
// |reason| indicates whether the transform was set directly or by an
// animation. This won't necessarily be called at every step of an animation.
// However, it will always be called before the first frame of the animation
// is rendered and when the animation ends. The client can determine whether
// the animation is ending by calling
// window->layer()->GetAnimator()->IsAnimatingProperty(
// ui::LayerAnimationElement::TRANSFORM).
virtual void OnWindowTransformed(Window* window,
Expand Down
4 changes: 2 additions & 2 deletions ui/aura/window_occlusion_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ void WindowOcclusionTracker::CleanupAnimatedWindows() {

bool WindowOcclusionTracker::MaybeObserveAnimatedWindow(Window* window) {
// MaybeObserveAnimatedWindow() is called when OnWindowBoundsChanged(),
// OnWindowTransformed() or OnWindowOpacityChanged() is called with
// OnWindowTransformed() or OnWindowOpacitySet() is called with
// ui::PropertyChangeReason::FROM_ANIMATION. Despite that, if the animation is
// ending, the IsAnimatingOnePropertyOf() call below may return false. It is
// important not to register an observer in that case because it would never
Expand Down Expand Up @@ -399,7 +399,7 @@ void WindowOcclusionTracker::OnWindowBoundsChanged(
});
}

void WindowOcclusionTracker::OnWindowOpacityChanged(
void WindowOcclusionTracker::OnWindowOpacitySet(
Window* window,
ui::PropertyChangeReason reason) {
// Call MaybeObserveAnimatedWindow() outside the lambda so that the window can
Expand Down
4 changes: 2 additions & 2 deletions ui/aura/window_occlusion_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds,
ui::PropertyChangeReason reason) override;
void OnWindowOpacityChanged(Window* window,
ui::PropertyChangeReason reason) override;
void OnWindowOpacitySet(Window* window,
ui::PropertyChangeReason reason) override;
void OnWindowTransformed(Window* window,
ui::PropertyChangeReason reason) override;
void OnWindowStackingChanged(Window* window) override;
Expand Down
8 changes: 4 additions & 4 deletions ui/aura/window_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1876,8 +1876,8 @@ class WindowObserverTest : public WindowTest,
window_bounds_info_.reason = reason;
}

void OnWindowOpacityChanged(Window* window,
ui::PropertyChangeReason reason) override {
void OnWindowOpacitySet(Window* window,
ui::PropertyChangeReason reason) override {
++window_opacity_info_.changed_count;
window_opacity_info_.window = window;
window_opacity_info_.reason = reason;
Expand Down Expand Up @@ -2094,7 +2094,7 @@ TEST_P(WindowObserverTest, WindowBoundsChangedAnimation) {
window_bounds_info().reason);
}

// Verify that WindowObserver::OnWindowOpacityChanged() is notified when the
// Verify that WindowObserver::OnWindowOpacitySet() is notified when the
// opacity of a Window's Layer changes without an animation.
TEST_P(WindowObserverTest, WindowOpacityChanged) {
std::unique_ptr<Window> window(CreateTestWindowWithId(1, root_window()));
Expand All @@ -2106,7 +2106,7 @@ TEST_P(WindowObserverTest, WindowOpacityChanged) {
window_opacity_info().reason);
}

// Verify that WindowObserver::OnWindowOpacityChanged() is notified at the
// Verify that WindowObserver::OnWindowOpacitySet() is notified at the
// beginning and at the end of a threaded opacity animation.
TEST_P(WindowObserverTest, WindowOpacityChangedAnimation) {
std::unique_ptr<Window> window(CreateTestWindowWithId(1, root_window()));
Expand Down

0 comments on commit 30b02ac

Please sign in to comment.