Handles users rotating screen too early

ash/rotator/screen_rotation_animator.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_ROTATOR_SCREEN_ROTATION_ANIMATOR_H_
 #define ASH_ROTATOR_SCREEN_ROTATION_ANIMATOR_H_
 
 #include <stdint.h>
 
 #include "ash/ash_export.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "ui/display/display.h"
 
+namespace ui {
+class Layer;
+class LayerTreeOwner;
+}  // namespace ui
+
 namespace ash {
+namespace test {
+class ScreenRotationAnimatorTestApi;
+}  // namespace test
+
+class ScreenRotationAnimatorObserver;
 
 // Utility to perform a screen rotation with an animation.
 class ASH_EXPORT ScreenRotationAnimator {
  public:
   explicit ScreenRotationAnimator(int64_t display_id);
   ~ScreenRotationAnimator();
 
-  // Returns true if the screen rotation animation can be completed
-  // successfully. For example an animation is not possible if |display_id_|
-  // specifies a display::Display that is not currently active. See
-  // www.crbug.com/479503.
-  bool CanAnimate() const;
-
   // Rotates the display::Display specified by |display_id_| to the
-  // |new_rotation|
-  // orientation, for the given |source|. The rotation will also become active.
-  // Clients should only call |Rotate()| if |CanAnimate()| returns true.
+  // |new_rotation| orientation, for the given |source|. The rotation will also
+  // become active. Clients should only call |Rotate()| when can animate. For
+  // example an animation is not possible if |display_id_| specifies a
+  // display::Display that is not currently active. See www.crbug.com/479503.

[bruthig, 2017/03/16 20:41:58]

nit: 'Clients should only call |Rotate()| when can animate. For example an
animation is not possible if |display_id_| specifies a display::Display that is
not currently active. See www.crbug.com/479503.' is some what confusing and
leaves the reader with more questions than it answers.  I think the following
would be better.

'Should only be called when |display_id_| references a valid display::Display.'

This makes me wonder if there is a race condition. i.e. If a display is removed,
like an external monitor, while there is a request pending, will it cause a
crash when processing the queue?

[wutao, 2017/03/17 00:01:03]

Right, in the ProcessAnimationQueue, I will add a check
if (last_pending_request_ && IsDisplayValid(display_id_))
  Rotate();

+  // |screen_rotation_animator_observer_| will be notified when rotation is
+  // finished and there is no more pending rotation request. Otherwise, any
+  // ongoging animation will be stoped and progressed to the target position,

[bruthig, 2017/03/16 20:41:57]

nit: 'ongoging' -> 'ongoing'

[wutao, 2017/03/17 00:01:03]

Done.

+  // followed by a new |Rotate()| call with the pending rotation request.
   void Rotate(display::Display::Rotation new_rotation,
               display::Display::RotationSource source);
 
+  int64_t display_id() const { return display_id_; }
+
+  void SetScreenRotationAnimatorObserver(
+      ScreenRotationAnimatorObserver* observer) {
+    screen_rotation_animator_observer_ = observer;
+  }
+
+  void RemoveScreenRotationAnimatorObserver() {
+    screen_rotation_animator_observer_ = nullptr;
+  }
+
+  void OnLayerAnimationEnded();
+
+  void OnLayerAnimationAborted();
+
  private:
+  friend class ash::test::ScreenRotationAnimatorTestApi;
+  struct ScreenRotationRequest;
+
+  // Set the screen orientation to |new_rotation| and animate the change. The
+  // animation will rotate the initial orientation's layer towards the new
+  // orientation through |rotation_degrees| while fading out, and the new
+  // orientation's layer will be rotated in to the |new_orientation| through
+  // |rotation_degrees| arc.
+  void AnimateRotation(const ScreenRotationRequest& rotation_request);
+
+  // Get the root layer of the old layer tree.
+  ui::Layer* GetOldLayerTreeRootLayer();
+
+  // When screen rotation animation is ended or aborted, if the request queue is
+  // not empty, it will call |Rotate()| with the pending rotation request.
+  // Otherwise it will notify |screen_rotation_animator_observer_|.
+  void ProcessAnimationQueue();
+
+  void set_disable_animation_timers_for_test(bool disable_timers);
+
+  void StopAnimating();
+
   // The id of the display to rotate.
   int64_t display_id_;
+  bool is_rotating_;
+  // Only set in unittest to disable animation timers.
+  bool disable_animation_timers_for_test_;
+  ScreenRotationAnimatorObserver* screen_rotation_animator_observer_;

[oshima, 2017/03/16 20:13:09]

us ObserverList and follow the same pattern as other observers.

[wutao, 2017/03/17 00:01:03]

Currently only one observer.
I will change to ObserverList though.

+  std::unique_ptr<ui::LayerTreeOwner> old_layer_tree_owner_;
+  std::unique_ptr<ScreenRotationRequest> last_pending_request_;
+  base::WeakPtrFactory<ScreenRotationAnimator> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimator);
 };
 
 }  // namespace ash
 
 #endif  // ASH_ROTATOR_SCREEN_ROTATION_ANIMATOR_H_