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 "ui/compositor/layer_tree_owner.h"
 #include "ui/display/display.h"
 
 namespace ash {
 
 // Utility to perform a screen rotation with an animation.
 class ASH_EXPORT ScreenRotationAnimator {
  public:
+  class ASH_EXPORT ScreenRotationAnimatorObserver {
+   public:
+    ScreenRotationAnimatorObserver();
+    virtual void OnEndedOrAbortedAnimation(ScreenRotationAnimator* animator);
+
+   protected:
+    ~ScreenRotationAnimatorObserver();

[bruthig, 2017/03/08 22:35:59]

This should be virtual because you expect classes to extend it.

[wutao, 2017/03/09 22:09:19]

Done.

+  };
+
+  struct ScreenRotationRequest {
+    ScreenRotationAnimator* animator;
+    int64_t display_id;
+    display::Display::Rotation initial_rotation;
+    display::Display::Rotation new_rotation;
+    display::Display::RotationSource source;
+  };
+
   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.
   void Rotate(display::Display::Rotation new_rotation,
               display::Display::RotationSource source);
 
+  int64_t display_id() const { return display_id_; };
+
+  bool is_rotating() const { return is_rotating_; };
+  void set_is_rotating(bool rotating) { is_rotating_ = rotating; };

[bruthig, 2017/03/08 22:35:59]

|is_rotating_| seems to me that it should be internal state of
ScreenRotationAnimator and as such shouldn't have public visibility.  I assume
you are doing this so that the LayerCleanupObserver can access it.

I see 2 (or 3) options here:

1) Move these functions to private and make the LayerCleanupObserver a friend
class.

2) Simply have the LayerCleanupObserver notify the ScreenRotationAnimator with a
general event like AnimationCompleted() and allow the animator to manage all of
it's internal state.

3) Similar to 2 but replace the LayerCleanupObserver with something like the
CallbackLayerAnimationObserver and have the animator own the LayerTreeOwner. 
The callback would handle deleting the layer tree and updating the animator's
internal state. (This would probably require the animator to maintain a
reference to the observer in order to safely complete destruction)

I have a strong preference for option 2 or 3 and I think either of them will
need the use of a WeakPtr.

[wutao, 2017/03/09 22:09:19]

(3) is almost close to what I implemented for the two screenshots version and I
planed to submit for review in the next next CLs. The reason why I did not use
CallbackLayerAnimationObserver to replace the LayerCleanupObserver here is try
to make this Cl smaller.

After talked offline with bruthig@, we plan to use option 2 here for now. But we
need to think about later how to handle two screenshots, need to wait two
animations to finish.

I will update the code soon.

+
+  ScreenRotationRequest* last_pending_request() const {
+    return last_pending_request_.get();
+  };
+
+  ScreenRotationAnimatorObserver* observer() const { return observer_; };
+  void SetObserver(ScreenRotationAnimatorObserver* observer) {
+    observer_ = observer;
+  };
+  void RemoveObserver() { observer_ = nullptr; };
+
+  ui::LayerTreeOwner* old_layer_tree_owner() const {
+    return old_layer_tree_owner_.get();
+  };
+  void reset_old_layer_tree_owner() { old_layer_tree_owner_.reset(); };
+  void set_old_layer_tree_owner(ui::LayerTreeOwner* layer_tree_owner) {
+    old_layer_tree_owner_.reset(layer_tree_owner);
+  };
+
  private:
   // The id of the display to rotate.
   int64_t display_id_;
+  bool is_rotating_;
+  ScreenRotationAnimatorObserver* observer_;
+  std::unique_ptr<ui::LayerTreeOwner> old_layer_tree_owner_;
+  std::unique_ptr<ScreenRotationRequest> last_pending_request_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenRotationAnimator);
 };
 
 }  // namespace ash
 
 #endif  // ASH_ROTATOR_SCREEN_ROTATION_ANIMATOR_H_