Issue 232133003: Fixing too long running unit tests on valgrind (produced by user switch animation)

Issue 232133003: Fixing too long running unit tests on valgrind (produced by user switch animation) (Closed)

Created:
6 years, 8 months ago by Mr4D (OOO till 08-26)

Modified:
6 years, 8 months ago

Reviewers:
Dmitry Polukhin

CC:
chromium-reviews, extensions-reviews_chromium.org, sadrul, nkostylev+watch_chromium.org, ben+ash_chromium.org, oshima+watch_chromium.org, kalyank, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Fixing too long running unit tests on valgrind (produced by user switch animation) BUG=361412 TEST=unittest Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263015

Patch Set 1 #

Total comments: 2

Patch Set 2 : . #

Patch Set 3 : self nit #

Created: 6 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+67 lines, -35 lines)			Patch
M	chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/chromeos/preferences_browsertest.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/ash/multi_user/multi_user_notification_blocker_chromeos_unittest.cc	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.h	View	1 2	4 chunks	+14 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc	View	1	6 chunks	+17 lines, -5 lines	0 comments	Download
M	chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc	View		7 chunks	+14 lines, -7 lines	0 comments	Download
M	chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc	View		6 chunks	+11 lines, -13 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Dmitry Polukhin

lgtm https://codereview.chromium.org/232133003/diff/1/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc File chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc (right): https://codereview.chromium.org/232133003/diff/1/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc#newcode693 chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc:693: (animation_speed_ == ANIMATION_SPEED_FAST ? 10 : 0)); I ...

6 years, 8 months ago (2014-04-10 00:11:48 UTC) #2

Mr4D (OOO till 08-26)

The CQ bit was checked by skuhne@chromium.org

6 years, 8 months ago (2014-04-10 14:34:31 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skuhne@chromium.org/232133003/30001

6 years, 8 months ago (2014-04-10 14:34:31 UTC) #4

Mr4D (OOO till 08-26)

6 years, 8 months ago (2014-04-10 17:10:17 UTC) #6

Message was sent while issue was closed.

Thanks for your review!

https://codereview.chromium.org/232133003/diff/1/chrome/browser/ui/ash/multi_...
File chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
(right):

https://codereview.chromium.org/232133003/diff/1/chrome/browser/ui/ash/multi_...
chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc:693:
(animation_speed_ == ANIMATION_SPEED_FAST ? 10 : 0));
On 2014/04/10 00:11:48, Dmitry Polukhin wrote:
> I would extract it to separate method that gets normal speed and return 10 or
0
> depending on animation_speed_. It will also slightly improve readability and
> compiler will inline this function.

Done.

Expand Messages | Collapse Messages