Commit 30ee9094 authored by rdevlin.cronin's avatar rdevlin.cronin Committed by Commit bot

[Mac Extensions Toolbar] Make the container's animationEndFrame always valid

Previously, [BrowserActionsContainer animationEndFrame] only returned the
frame after animation once animation had occurred. Replace this with an always-
valid animationEndFrame method that returns the frame after animation, or the
current frame if the container is not animating.

Also, move the animation suppression for testing to the ToolbarActionsBar
so that it is platform-independent.

TBR=sky@chromium.org (moving animation suppression flag)
BUG=435518

Review URL: https://codereview.chromium.org/795663003

Cr-Commit-Position: refs/heads/master@{#308190}
parent 9f03ba31
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "chrome/browser/extensions/extension_toolbar_model.h" #include "chrome/browser/extensions/extension_toolbar_model.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h" #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
#import "chrome/browser/ui/cocoa/browser_window_cocoa.h" #import "chrome/browser/ui/cocoa/browser_window_cocoa.h"
#import "chrome/browser/ui/cocoa/browser_window_controller.h" #import "chrome/browser/ui/cocoa/browser_window_controller.h"
#import "chrome/browser/ui/cocoa/extensions/browser_action_button.h" #import "chrome/browser/ui/cocoa/extensions/browser_action_button.h"
...@@ -118,10 +119,12 @@ bool BrowserActionTestUtil::HidePopup() { ...@@ -118,10 +119,12 @@ bool BrowserActionTestUtil::HidePopup() {
// static // static
void BrowserActionTestUtil::DisableAnimations() { void BrowserActionTestUtil::DisableAnimations() {
ToolbarActionsBar::disable_animations_for_testing_ = true;
} }
// static // static
void BrowserActionTestUtil::EnableAnimations() { void BrowserActionTestUtil::EnableAnimations() {
ToolbarActionsBar::disable_animations_for_testing_ = false;
} }
// static // static
......
...@@ -32,10 +32,6 @@ extern NSString* const kTranslationWithDelta; ...@@ -32,10 +32,6 @@ extern NSString* const kTranslationWithDelta;
// The frame encompasing the grippy used for resizing the container. // The frame encompasing the grippy used for resizing the container.
NSRect grippyRect_; NSRect grippyRect_;
// The end frame of the animation currently running for this container or
// NSZeroRect if none is in progress.
NSRect animationEndFrame_;
// Used to cache the original position within the container that initiated the // Used to cache the original position within the container that initiated the
// drag. // drag.
NSPoint initialDragPoint_; NSPoint initialDragPoint_;
...@@ -79,7 +75,10 @@ extern NSString* const kTranslationWithDelta; ...@@ -79,7 +75,10 @@ extern NSString* const kTranslationWithDelta;
// placement of surrounding elements. // placement of surrounding elements.
- (CGFloat)resizeDeltaX; - (CGFloat)resizeDeltaX;
@property(nonatomic, readonly) NSRect animationEndFrame; // Returns the frame of the container after the running animation has finished.
// If no animation is running, returns the container's current frame.
- (NSRect)animationEndFrame;
@property(nonatomic) BOOL canDragLeft; @property(nonatomic) BOOL canDragLeft;
@property(nonatomic) BOOL canDragRight; @property(nonatomic) BOOL canDragRight;
@property(nonatomic) BOOL grippyPinned; @property(nonatomic) BOOL grippyPinned;
......
...@@ -36,7 +36,6 @@ const CGFloat kMinimumContainerWidth = 10.0; ...@@ -36,7 +36,6 @@ const CGFloat kMinimumContainerWidth = 10.0;
@implementation BrowserActionsContainerView @implementation BrowserActionsContainerView
@synthesize animationEndFrame = animationEndFrame_;
@synthesize canDragLeft = canDragLeft_; @synthesize canDragLeft = canDragLeft_;
@synthesize canDragRight = canDragRight_; @synthesize canDragRight = canDragRight_;
@synthesize grippyPinned = grippyPinned_; @synthesize grippyPinned = grippyPinned_;
...@@ -174,7 +173,6 @@ const CGFloat kMinimumContainerWidth = 10.0; ...@@ -174,7 +173,6 @@ const CGFloat kMinimumContainerWidth = 10.0;
[[NSAnimationContext currentContext] setDuration:kAnimationDuration]; [[NSAnimationContext currentContext] setDuration:kAnimationDuration];
[[self animator] setFrame:newFrame]; [[self animator] setFrame:newFrame];
[NSAnimationContext endGrouping]; [NSAnimationContext endGrouping];
animationEndFrame_ = newFrame;
[[NSNotificationCenter defaultCenter] [[NSNotificationCenter defaultCenter]
postNotificationName:kBrowserActionsContainerWillAnimate postNotificationName:kBrowserActionsContainerWillAnimate
...@@ -189,6 +187,10 @@ const CGFloat kMinimumContainerWidth = 10.0; ...@@ -189,6 +187,10 @@ const CGFloat kMinimumContainerWidth = 10.0;
return [self frame].origin.x - lastXPos_; return [self frame].origin.x - lastXPos_;
} }
- (NSRect)animationEndFrame {
return [[self animator] frame];
}
#pragma mark - #pragma mark -
#pragma mark Private Methods #pragma mark Private Methods
......
...@@ -669,11 +669,8 @@ class NotificationBridge : public WrenchMenuBadgeController::Delegate { ...@@ -669,11 +669,8 @@ class NotificationBridge : public WrenchMenuBadgeController::Delegate {
CGFloat edgeXPos = [wrenchButton_ frame].origin.x; CGFloat edgeXPos = [wrenchButton_ frame].origin.x;
leftDistance = edgeXPos - locationBarXPos - kWrenchMenuLeftPadding; leftDistance = edgeXPos - locationBarXPos - kWrenchMenuLeftPadding;
} else { } else {
NSRect containerFrame = animate ? leftDistance = NSMinX([browserActionsContainerView_ animationEndFrame]) -
[browserActionsContainerView_ animationEndFrame] : locationBarXPos;
[browserActionsContainerView_ frame];
leftDistance = containerFrame.origin.x - locationBarXPos;
} }
if (leftDistance != 0.0) if (leftDistance != 0.0)
[self adjustLocationSizeBy:leftDistance animate:animate]; [self adjustLocationSizeBy:leftDistance animate:animate];
......
...@@ -61,6 +61,9 @@ int GetIconDimension(DimensionType type) { ...@@ -61,6 +61,9 @@ int GetIconDimension(DimensionType type) {
} // namespace } // namespace
// static
bool ToolbarActionsBar::disable_animations_for_testing_ = false;
ToolbarActionsBar::PlatformSettings::PlatformSettings(bool in_overflow_mode) ToolbarActionsBar::PlatformSettings::PlatformSettings(bool in_overflow_mode)
: left_padding(in_overflow_mode ? kOverflowLeftPadding : kLeftPadding), : left_padding(in_overflow_mode ? kOverflowLeftPadding : kLeftPadding),
right_padding(in_overflow_mode ? kOverflowRightPadding : kRightPadding), right_padding(in_overflow_mode ? kOverflowRightPadding : kRightPadding),
...@@ -525,6 +528,7 @@ void ToolbarActionsBar::OnToolbarModelInitialized() { ...@@ -525,6 +528,7 @@ void ToolbarActionsBar::OnToolbarModelInitialized() {
// We shouldn't have any actions before the model is initialized. // We shouldn't have any actions before the model is initialized.
DCHECK(toolbar_actions_.empty()); DCHECK(toolbar_actions_.empty());
CreateActions(); CreateActions();
ResizeDelegate(gfx::Tween::EASE_OUT, false);
} }
Browser* ToolbarActionsBar::GetBrowser() { Browser* ToolbarActionsBar::GetBrowser() {
......
...@@ -119,7 +119,9 @@ class ToolbarActionsBar : public extensions::ExtensionToolbarModel::Observer, ...@@ -119,7 +119,9 @@ class ToolbarActionsBar : public extensions::ExtensionToolbarModel::Observer,
} }
bool enabled() const { return model_ != nullptr; } bool enabled() const { return model_ != nullptr; }
bool suppress_layout() const { return suppress_layout_; } bool suppress_layout() const { return suppress_layout_; }
bool suppress_animation() const { return suppress_animation_; } bool suppress_animation() const {
return suppress_animation_ || disable_animations_for_testing_;
}
bool is_highlighting() const { return model_ && model_->is_highlighting(); } bool is_highlighting() const { return model_ && model_->is_highlighting(); }
const PlatformSettings& platform_settings() const { const PlatformSettings& platform_settings() const {
return platform_settings_; return platform_settings_;
...@@ -127,6 +129,11 @@ class ToolbarActionsBar : public extensions::ExtensionToolbarModel::Observer, ...@@ -127,6 +129,11 @@ class ToolbarActionsBar : public extensions::ExtensionToolbarModel::Observer,
ToolbarActionsBarDelegate* delegate_for_test() { return delegate_; } ToolbarActionsBarDelegate* delegate_for_test() { return delegate_; }
// During testing we can disable animations by setting this flag to true,
// so that the bar resizes instantly, instead of having to poll it while it
// animates to open/closed status.
static bool disable_animations_for_testing_;
private: private:
using ToolbarActions = ScopedVector<ToolbarActionViewController>; using ToolbarActions = ScopedVector<ToolbarActionViewController>;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/browser_window_testing_views.h" #include "chrome/browser/ui/browser_window_testing_views.h"
#include "chrome/browser/ui/extensions/extension_action_view_controller.h" #include "chrome/browser/ui/extensions/extension_action_view_controller.h"
#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
#include "chrome/browser/ui/views/extensions/extension_popup.h" #include "chrome/browser/ui/views/extensions/extension_popup.h"
#include "chrome/browser/ui/views/toolbar/browser_actions_container.h" #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
#include "chrome/browser/ui/views/toolbar/toolbar_action_view.h" #include "chrome/browser/ui/views/toolbar/toolbar_action_view.h"
...@@ -113,12 +114,12 @@ void BrowserActionTestUtil::SetIconVisibilityCount(size_t icons) { ...@@ -113,12 +114,12 @@ void BrowserActionTestUtil::SetIconVisibilityCount(size_t icons) {
// static // static
void BrowserActionTestUtil::DisableAnimations() { void BrowserActionTestUtil::DisableAnimations() {
BrowserActionsContainer::disable_animations_during_testing_ = true; ToolbarActionsBar::disable_animations_for_testing_ = true;
} }
// static // static
void BrowserActionTestUtil::EnableAnimations() { void BrowserActionTestUtil::EnableAnimations() {
BrowserActionsContainer::disable_animations_during_testing_ = false; ToolbarActionsBar::disable_animations_for_testing_ = false;
} }
// static // static
......
...@@ -63,9 +63,6 @@ BrowserActionsContainer::DropPosition::DropPosition( ...@@ -63,9 +63,6 @@ BrowserActionsContainer::DropPosition::DropPosition(
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// BrowserActionsContainer // BrowserActionsContainer
// static
bool BrowserActionsContainer::disable_animations_during_testing_ = false;
BrowserActionsContainer::BrowserActionsContainer( BrowserActionsContainer::BrowserActionsContainer(
Browser* browser, Browser* browser,
BrowserActionsContainer* main_container) BrowserActionsContainer* main_container)
...@@ -288,9 +285,7 @@ void BrowserActionsContainer::ResizeAndAnimate( ...@@ -288,9 +285,7 @@ void BrowserActionsContainer::ResizeAndAnimate(
gfx::Tween::Type tween_type, gfx::Tween::Type tween_type,
int target_width, int target_width,
bool suppress_chevron) { bool suppress_chevron) {
if (resize_animation_ && if (resize_animation_ && !toolbar_actions_bar_->suppress_animation()) {
!disable_animations_during_testing_ &&
!toolbar_actions_bar_->suppress_animation()) {
// Animate! We have to set the animation_target_size_ after calling Reset(), // Animate! We have to set the animation_target_size_ after calling Reset(),
// because that could end up calling AnimationEnded which clears the value. // because that could end up calling AnimationEnded which clears the value.
resize_animation_->Reset(); resize_animation_->Reset();
......
...@@ -255,11 +255,6 @@ class BrowserActionsContainer ...@@ -255,11 +255,6 @@ class BrowserActionsContainer
// Retrieve the current popup. This should only be used by unit tests. // Retrieve the current popup. This should only be used by unit tests.
gfx::NativeView TestGetPopup(); gfx::NativeView TestGetPopup();
// During testing we can disable animations by setting this flag to true,
// so that the bar resizes instantly, instead of having to poll it while it
// animates to open/closed status.
static bool disable_animations_during_testing_;
protected: protected:
// Overridden from views::View: // Overridden from views::View:
void ViewHierarchyChanged( void ViewHierarchyChanged(
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
#include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/test_with_browser_view.h" #include "chrome/browser/ui/views/frame/test_with_browser_view.h"
#include "chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.h" #include "chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.h"
...@@ -126,7 +127,7 @@ void ToolbarViewInteractiveUITest::SetUpCommandLine( ...@@ -126,7 +127,7 @@ void ToolbarViewInteractiveUITest::SetUpCommandLine(
// are constructed. // are constructed.
feature_override_.reset(new extensions::FeatureSwitch::ScopedOverride( feature_override_.reset(new extensions::FeatureSwitch::ScopedOverride(
extensions::FeatureSwitch::extension_action_redesign(), true)); extensions::FeatureSwitch::extension_action_redesign(), true));
BrowserActionsContainer::disable_animations_during_testing_ = true; ToolbarActionsBar::disable_animations_for_testing_ = true;
WrenchToolbarButton::g_open_wrench_immediately_for_testing = true; WrenchToolbarButton::g_open_wrench_immediately_for_testing = true;
} }
...@@ -138,7 +139,7 @@ void ToolbarViewInteractiveUITest::SetUpOnMainThread() { ...@@ -138,7 +139,7 @@ void ToolbarViewInteractiveUITest::SetUpOnMainThread() {
} }
void ToolbarViewInteractiveUITest::TearDownOnMainThread() { void ToolbarViewInteractiveUITest::TearDownOnMainThread() {
BrowserActionsContainer::disable_animations_during_testing_ = false; ToolbarActionsBar::disable_animations_for_testing_ = false;
WrenchToolbarButton::g_open_wrench_immediately_for_testing = false; WrenchToolbarButton::g_open_wrench_immediately_for_testing = false;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment