[Mac] Refactor the fullscreen toolbar animation

chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_animation_controller.h

+// Copyright 2016 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 CHROME_BROWSER_UI_COCOA_FULLSCREEN_FULLSCREEN_TOOLBAR_ANIMATION_CONTROLLER_H_
+#define CHROME_BROWSER_UI_COCOA_FULLSCREEN_FULLSCREEN_TOOLBAR_ANIMATION_CONTROLLER_H_
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/timer/timer.h"
+#include "ui/gfx/animation/animation_delegate.h"
+#include "ui/gfx/animation/slide_animation.h"
+
+namespace {

[erikchen, 2016/10/21 17:22:03]

don't use anonymous namespaces in header files. Use FRIEND_TEST_ALL_PREFIXES
instead.

[spqchan, 2016/10/21 18:26:55]

Removed

+class FullscreenToolbarLayoutTest;
+}
+
+@class FullscreenToolbarController;
+
+class FullscreenToolbarAnimationController : public gfx::AnimationDelegate {
+ public:
+  FullscreenToolbarAnimationController(FullscreenToolbarController* owner);
+
+  // Called by |owner_| to update the current toolbar fraction.
+  void UpdateToolbarFraction(CGFloat fraction);
+
+  // Stops the animation and cancels |hide_toolbar_timer_|.
+  void StopAnimationAndTimer();
+
+  // Animates the toolbar in and out to show changes with the tabstrip.
+  void AnimateToolbarForTabstripChanges();
+
+  // Animates the toolbar in if it's not fully shown.
+  void AnimateToolbarIn();
+
+  // Animates the toolbar out if it's not focused.
+  void AnimateToolbarOutIfPossible();
+
+  // Returns the toolbar fraction calculated from the animation progress.
+  CGFloat GetToolbarFraction() const;
+
+  // Returns true if |animation_| is running.
+  BOOL IsAnimationRunning() const;
+
+  // gfx::AnimationDelegate:
+  void AnimationProgressed(const gfx::Animation* animation) override;
+  void AnimationEnded(const gfx::Animation* animation) override;
+
+ private:
+  friend class ::FullscreenToolbarLayoutTest;
+
+  // Our owner.
+  FullscreenToolbarController* owner_;  // weak
+
+  // The animation of the decoration.
+  gfx::SlideAnimation animation_;
+
+  // Timer that will start the scrollbar's hiding animation when it reaches 0.
+  base::Timer hide_toolbar_timer_;
+
+  // The current fraction of the toolbar being displayed. Used as the starting
+  // point of the animation.
+  CGFloat toolbar_fraction_;
+
+  // True when the toolbar is dropped to show tabstrip changes.
+  BOOL should_hide_toolbar_after_delay_;
+
+  // Used to disable find bar animations when testing.
+  BOOL disable_animations_during_testing_;

[erikchen, 2016/10/21 17:22:03]

I notice this isn't used yet. Given that this CL is part of a series of
refactors, I'm okay with the temporary presence of an unused variable.

That being said, changing behavior for tests to a large extension defeats the
point of testing [since you're no longer testing the same behavior]. I recommend
instead that all of your animate methods have a parameter BOOL:animated [This is
what appkit does] that changes the animation duration between N and 0. This way,
you can still test animations, just with a duration of 0.

[spqchan, 2016/10/21 18:26:55]

Removed this since it looks like the tests need to be change and I don't want to
have a lot of overhead.
I'm not sure what you mean. Do you mean something like AnimateToolbarIn(bool
should_animate)?

[erikchen, 2016/10/21 19:36:59]

yeah, but that wording is a little bit awkward. Maybe AnimateToolbarIn(bool
instantaneous)

[spqchan, 2016/10/21 20:34:18]

Ah, gotcha. Parameters won't be necessary for this case since the friend tests
can directly set the duration.

+};
+
+#endif  // CHROME_BROWSER_UI_COCOA_FULLSCREEN_FULLSCREEN_TOOLBAR_ANIMATION_CONTROLLER_H_