Skip to content

Commit

Permalink
Avoid msan test crash on expand arrow button
Browse files Browse the repository at this point in the history
Changes:
Msan test crash occur when ChromeVox is enabled and expand arrow button
hitting animation is running. The crash may be caused by false positive
from Memory sanitizer (catchorg/Catch2#731).
To avoid such crash and unblock adding more ChromeVox tests, disable the
animation for app list during test.

Bug: 926038
Change-Id: I5c775388cb96443ffe4462c870dd023609d3ddb8
Reviewed-on: https://chromium-review.googlesource.com/c/1471499
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632287}
  • Loading branch information
Weidong Guo authored and Commit Bot committed Feb 14, 2019
1 parent 96fc7a3 commit 6ac1478
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
8 changes: 6 additions & 2 deletions ash/app_list/views/expand_arrow_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,13 @@ ExpandArrowView::ExpandArrowView(ContentsView* contents_view,
animation_->SetSlideDuration(kCycleDurationInMs * 2 + kCycleIntervalInMs);
ResetHintingAnimation();
// When side shelf or tablet mode is enabled, the peeking launcher won't be
// shown, so the hint animation is unnecessary.
if (!app_list_view_->is_side_shelf() && !app_list_view_->is_tablet_mode())
// shown, so the hint animation is unnecessary. Also, do not run the animation
// during test since we are not testing the animation and it might cause msan
// crash when spoken feedbacke is enabled (See https://crbug.com/926038).
if (!app_list_view_->is_side_shelf() && !app_list_view_->is_tablet_mode() &&
!AppListView::ShortAnimationsForTesting()) {
ScheduleHintingAnimation(true);
}
}

ExpandArrowView::~ExpandArrowView() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <queue>

#include "ash/accelerators/accelerator_controller.h"
#include "ash/app_list/views/app_list_view.h"
#include "ash/public/cpp/accelerators.h"
#include "ash/root_window_controller.h"
#include "ash/shell.h"
Expand Down Expand Up @@ -349,8 +350,10 @@ IN_PROC_BROWSER_TEST_P(SpokenFeedbackTest, DISABLED_TypeInOmnibox) {
EXPECT_EQ("z", speech_monitor_.GetNextUtterance());
}

// TODO(https://crbug.com/926038): Failed MSAN test
IN_PROC_BROWSER_TEST_P(SpokenFeedbackTest, DISABLED_LauncherStateTransition) {
// Do not running expand arrow hinting animation to avoid msan test crash.
// (See https://crbug.com/926038)
app_list::AppListView::SetShortAnimationForTesting(true);
EnableChromeVox();

EXPECT_TRUE(PerformAcceleratorAction(ash::FOCUS_SHELF));
Expand Down Expand Up @@ -389,11 +392,14 @@ IN_PROC_BROWSER_TEST_P(SpokenFeedbackTest, DISABLED_LauncherStateTransition) {

// Check that Launcher, all apps state is announced.
EXPECT_EQ("Launcher, all apps", speech_monitor_.GetNextUtterance());
app_list::AppListView::SetShortAnimationForTesting(false);
}

// TODO(https://crbug.com/926038): Failed MSAN test
IN_PROC_BROWSER_TEST_P(SpokenFeedbackTest,
DISABLED_DisabledFullscreenExpandButton) {
// Do not running expand arrow hinting animation to avoid msan test crash.
// (See https://crbug.com/926038)
app_list::AppListView::SetShortAnimationForTesting(true);
EnableChromeVox();

EXPECT_TRUE(PerformAcceleratorAction(ash::FOCUS_SHELF));
Expand Down Expand Up @@ -425,6 +431,7 @@ IN_PROC_BROWSER_TEST_P(SpokenFeedbackTest,
// Make sure the second traversal left is not the expand arrow button.
SendKeyPressWithSearch(ui::VKEY_LEFT);
EXPECT_NE("Expand to all apps", speech_monitor_.GetNextUtterance());
app_list::AppListView::SetShortAnimationForTesting(false);
}

IN_PROC_BROWSER_TEST_P(SpokenFeedbackTest, FocusShelf) {
Expand Down

0 comments on commit 6ac1478

Please sign in to comment.