Skip to content

Commit 020c1c4

Browse files
committed
[merge-m69] MacViews: Don't animate in web-modals after the first Show().
Implement the currently unimplemented (on mac) NativeWidget:: SetVisibilityAnimationTransition() for this. Autofill uses this but not constrained windows. Constrained windows on aura platforms have bespoke animation suppression code, which is currently different to the desired behavior on Mac (i.e. the very first "show" should animate). Consolidate some of the animation logic in MacViews Widgets. [email protected] (cherry picked from commit 7f4f6cf) Bug: 767743 Change-Id: If49d139a66873e896e68030f69efdaa81a8dc948 Reviewed-on: https://chromium-review.googlesource.com/1146408 Reviewed-by: Michael Wasserman <[email protected]> Reviewed-by: Sidney San Martín <[email protected]> Commit-Queue: Trent Apted <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#577343} Reviewed-on: https://chromium-review.googlesource.com/1150016 Reviewed-by: Trent Apted <[email protected]> Cr-Commit-Position: refs/branch-heads/3497@{#94} Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
1 parent 06534e4 commit 020c1c4

File tree

5 files changed

+64
-20
lines changed

5 files changed

+64
-20
lines changed

components/constrained_window/native_web_contents_modal_dialog_manager_views.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ void NativeWebContentsModalDialogManagerViews::Show() {
106106
// shadows are below the constrained dialog in z-order when we do this.
107107
shown_widgets_.insert(widget);
108108
#endif
109+
110+
#if !defined(USE_AURA)
111+
// Don't re-animate when switching tabs. Note this is done on Mac only after
112+
// the initial ShowWidget() call above, and then "sticks" for later calls.
113+
// TODO(tapted): Consolidate this codepath with Aura.
114+
widget->SetVisibilityAnimationTransition(views::Widget::ANIMATE_HIDE);
115+
#endif
109116
}
110117

111118
void NativeWebContentsModalDialogManagerViews::Hide() {

ui/views/cocoa/bridged_native_widget.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,19 @@ class VIEWS_EXPORT BridgedNativeWidget
213213
bool wants_to_be_visible() const { return wants_to_be_visible_; }
214214
bool in_fullscreen_transition() const { return in_fullscreen_transition_; }
215215

216-
bool GetAnimate() const;
217-
void SetAnimate(bool animate);
216+
// Enables or disables all window animations.
217+
void SetAnimationEnabled(bool animate);
218+
219+
// Sets which transitions will animate. Currently this only affects non-native
220+
// animations. TODO(tapted): Use scoping to disable native animations at
221+
// appropriate times as well.
222+
void set_transitions_to_animate(int transitions) {
223+
transitions_to_animate_ = transitions;
224+
}
225+
226+
// Whether to run a custom animation for the provided |transition|.
227+
bool ShouldRunCustomAnimationFor(
228+
Widget::VisibilityTransition transition) const;
218229

219230
// ui::CATransactionCoordinator::PreCommitObserver implementation
220231
bool ShouldWaitInPreCommit() override;
@@ -326,6 +337,10 @@ class VIEWS_EXPORT BridgedNativeWidget
326337
// window in AppKit coordinates is not actually changing in this case.
327338
gfx::Point last_window_frame_origin_;
328339

340+
// The transition types to animate when not relying on native NSWindow
341+
// animation behaviors. Bitmask of Widget::VisibilityTransition.
342+
int transitions_to_animate_ = Widget::ANIMATE_BOTH;
343+
329344
// Whether this window wants to be fullscreen. If a fullscreen animation is in
330345
// progress then it might not be actually fullscreen.
331346
bool target_fullscreen_state_;

ui/views/cocoa/bridged_native_widget.mm

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <stddef.h>
99
#include <stdint.h>
1010

11+
#include "base/command_line.h"
1112
#include "base/logging.h"
1213
#import "base/mac/foundation_util.h"
1314
#include "base/mac/mac_util.h"
@@ -21,6 +22,7 @@
2122
#include "ui/base/ime/input_method.h"
2223
#include "ui/base/ime/input_method_factory.h"
2324
#include "ui/base/layout.h"
25+
#include "ui/base/ui_base_switches.h"
2426
#include "ui/compositor/recyclable_compositor_mac.h"
2527
#include "ui/gfx/geometry/dip_util.h"
2628
#import "ui/gfx/mac/coordinate_conversion.h"
@@ -527,7 +529,7 @@ NSUInteger CountBridgedWindows(NSArray* child_windows) {
527529

528530
// For non-sheet modal types, use the constrained window animations to make
529531
// the window appear.
530-
if (GetAnimate() && native_widget_mac_->GetWidget()->IsModal()) {
532+
if (ShouldRunCustomAnimationFor(Widget::ANIMATE_SHOW)) {
531533
show_animation_.reset(
532534
[[ModalShowAnimationWithLayer alloc] initWithBridgedNativeWidget:this]);
533535

@@ -994,16 +996,31 @@ NSUInteger CountBridgedWindows(NSArray* child_windows) {
994996
}
995997
}
996998

997-
bool BridgedNativeWidget::GetAnimate() const {
998-
return [window_ animationBehavior] != NSWindowAnimationBehaviorNone;
999-
}
1000-
1001-
void BridgedNativeWidget::SetAnimate(bool animate) {
999+
void BridgedNativeWidget::SetAnimationEnabled(bool animate) {
10021000
[window_
10031001
setAnimationBehavior:(animate ? NSWindowAnimationBehaviorDocumentWindow
10041002
: NSWindowAnimationBehaviorNone)];
10051003
}
10061004

1005+
bool BridgedNativeWidget::ShouldRunCustomAnimationFor(
1006+
Widget::VisibilityTransition transition) const {
1007+
// The logic around this needs to change if new transition types are set.
1008+
// E.g. it would be nice to distinguish "hide" from "close". Mac currently
1009+
// treats "hide" only as "close". Hide (e.g. Cmd+h) should not animate on Mac.
1010+
constexpr int kSupported =
1011+
Widget::ANIMATE_SHOW | Widget::ANIMATE_HIDE | Widget::ANIMATE_NONE;
1012+
DCHECK_EQ(0, transitions_to_animate_ & ~kSupported);
1013+
1014+
// Custom animations are only used for tab-modals. Note this also checks the
1015+
// native animation property. Clearing that will also disable custom
1016+
// animations to ensure that the views::Widget API behaves consistently.
1017+
return (transitions_to_animate_ & transition) &&
1018+
native_widget_mac_->GetWidget()->IsModal() &&
1019+
[window_ animationBehavior] != NSWindowAnimationBehaviorNone &&
1020+
!base::CommandLine::ForCurrentProcess()->HasSwitch(
1021+
switches::kDisableModalAnimations);
1022+
}
1023+
10071024
////////////////////////////////////////////////////////////////////////////////
10081025
// BridgedNativeWidget, ui::CATransactionObserver
10091026

ui/views/widget/native_widget_mac.mm

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,13 @@
99
#include <utility>
1010

1111
#include "base/bind.h"
12-
#include "base/command_line.h"
1312
#include "base/mac/foundation_util.h"
1413
#include "base/mac/scoped_nsobject.h"
1514
#include "base/strings/sys_string_conversions.h"
1615
#include "base/threading/thread_task_runner_handle.h"
1716
#include "components/crash/core/common/crash_key.h"
1817
#import "ui/base/cocoa/constrained_window/constrained_window_animation.h"
1918
#import "ui/base/cocoa/window_size_constants.h"
20-
#include "ui/base/ui_base_switches.h"
2119
#include "ui/display/display.h"
2220
#include "ui/display/screen.h"
2321
#include "ui/gfx/font_list.h"
@@ -50,11 +48,6 @@ + (void)closeWindowWithAnimation:(NSWindow*)window;
5048
namespace views {
5149
namespace {
5250

53-
bool AreModalAnimationsEnabled() {
54-
return !base::CommandLine::ForCurrentProcess()->HasSwitch(
55-
switches::kDisableModalAnimations);
56-
}
57-
5851
NSInteger StyleMaskForParams(const Widget::InitParams& params) {
5952
// If the Widget is modal, it will be displayed as a sheet. This works best if
6053
// it has NSTitledWindowMask. For example, with NSBorderlessWindowMask, the
@@ -402,8 +395,7 @@ NSInteger StyleMaskForParams(const Widget::InitParams& params) {
402395
}
403396

404397
// For other modal types, animate the close.
405-
if (bridge_->GetAnimate() && AreModalAnimationsEnabled() &&
406-
delegate_->IsModal()) {
398+
if (bridge_->ShouldRunCustomAnimationFor(Widget::ANIMATE_HIDE)) {
407399
[ViewsNSWindowCloseAnimator closeWindowWithAnimation:window];
408400
return;
409401
}
@@ -634,7 +626,7 @@ NSInteger StyleMaskForParams(const Widget::InitParams& params) {
634626

635627
void NativeWidgetMac::SetVisibilityChangedAnimationsEnabled(bool value) {
636628
if (bridge_)
637-
bridge_->SetAnimate(value);
629+
bridge_->SetAnimationEnabled(value);
638630
}
639631

640632
void NativeWidgetMac::SetVisibilityAnimationDuration(
@@ -644,7 +636,8 @@ NSInteger StyleMaskForParams(const Widget::InitParams& params) {
644636

645637
void NativeWidgetMac::SetVisibilityAnimationTransition(
646638
Widget::VisibilityTransition transition) {
647-
NOTIMPLEMENTED();
639+
if (bridge_)
640+
bridge_->set_transitions_to_animate(transition);
648641
}
649642

650643
bool NativeWidgetMac::IsTranslucentWindowOpacitySupported() const {

ui/views/widget/native_widget_mac_unittest.mm

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1218,12 +1218,24 @@ new ModalDialogDelegate(ui::MODAL_TYPE_CHILD), nullptr,
12181218
retained_animation.reset();
12191219

12201220
// Disable animations and show again.
1221-
modal_dialog_widget->SetVisibilityChangedAnimationsEnabled(false);
1221+
modal_dialog_widget->SetVisibilityAnimationTransition(Widget::ANIMATE_NONE);
12221222
modal_dialog_widget->Show();
12231223
EXPECT_FALSE(test_api.show_animation()); // No animation this time.
12241224
modal_dialog_widget->Hide();
12251225

12261226
// Test after re-enabling.
1227+
modal_dialog_widget->SetVisibilityAnimationTransition(Widget::ANIMATE_BOTH);
1228+
modal_dialog_widget->Show();
1229+
EXPECT_TRUE(test_api.show_animation());
1230+
retained_animation.reset(test_api.show_animation(),
1231+
base::scoped_policy::RETAIN);
1232+
1233+
// Test whether disabling native animations also disables custom modal ones.
1234+
modal_dialog_widget->SetVisibilityChangedAnimationsEnabled(false);
1235+
modal_dialog_widget->Show();
1236+
EXPECT_FALSE(test_api.show_animation()); // No animation this time.
1237+
modal_dialog_widget->Hide();
1238+
// Renable.
12271239
modal_dialog_widget->SetVisibilityChangedAnimationsEnabled(true);
12281240
modal_dialog_widget->Show();
12291241
EXPECT_TRUE(test_api.show_animation());

0 commit comments

Comments
 (0)