New issue
Advanced search Search tips

Issue 619344 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 634128



Sign in to add a comment

VisibleBounds/ShelfViewVisibleBoundsTest fails on trybots when system update icon is hidden

Project Member Reported by jamescook@chromium.org, Jun 11 2016

Issue description

I recently changed the system update icon to be hidden in tests. It used to default to visible via DefaultSystemTrayDelegate.

When I did this VisibleBounds/ShelfViewVisibleBoundsTest started to fail. The width is too small by 8 pixels. However, it only fails on the trybots -- I cannot repro locally.

Something is wrong in the shelf view preferred bounds computation.

VisibleBounds/ShelfViewVisibleBoundsTest.ItemsAreInBounds/0 (run #1):
[ RUN      ] VisibleBounds/ShelfViewVisibleBoundsTest.ItemsAreInBounds/0
[13843:13843:0611/103242:6165395337:ERROR:shelf_view_unittest.cc(1980)] with space
[13843:13843:0611/103242:6165395401:ERROR:shelf_view_unittest.cc(1950)] JAMES visible_bounds 0,553 268x47
[13843:13843:0611/103242:6165395440:ERROR:shelf_view_unittest.cc(1952)] JAMES shelf_bounds 0,553 638x47
[13843:13843:0611/103243:6165625403:ERROR:shelf_view_unittest.cc(1987)] with overflow
[13843:13843:0611/103243:6165625436:ERROR:shelf_view_unittest.cc(1950)] JAMES visible_bounds 0,553 646x47
[13843:13843:0611/103243:6165625451:ERROR:shelf_view_unittest.cc(1952)] JAMES shelf_bounds 0,553 638x47
../../ash/shelf/shelf_view_unittest.cc:1953: Failure
Value of: shelf_bounds.Contains(visible_bounds)
  Actual: false
Expected: true

 
Components: Tests>Disabled
I worked around this by forcing the system update bubble to be shown in this case.

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b74d0d9f84a249083ff6541bf478d18a7bd8dee

commit 3b74d0d9f84a249083ff6541bf478d18a7bd8dee
Author: mohsen <mohsen@chromium.org>
Date: Thu Oct 27 21:23:13 2016

Disable ShelfViewVisibleBoundsTest flaky tests

Temporarily disable flaky tests in ShelfViewVisibleBoundsTest, until the
fix for issue 634128 lands which hopefully fixes this flake, too.

BUG=625671, 619344 

Review-Url: https://codereview.chromium.org/2457903002
Cr-Commit-Position: refs/heads/master@{#428141}

[modify] https://crrev.com/3b74d0d9f84a249083ff6541bf478d18a7bd8dee/ash/shelf/shelf_view_unittest.cc

Comment 4 by moh...@chromium.org, Oct 27 2016

Blockedon: 634128
r428141 disables the test. Hopefully, fix for issue 634128 fixes this test properly.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/888d1c0b366685696e5f4c73775575cc61bdab87

commit 888d1c0b366685696e5f4c73775575cc61bdab87
Author: mohsen <mohsen@chromium.org>
Date: Tue Nov 22 04:56:51 2016

Fix spacing issues on Ash shelf

Following issues with shelf spacing are fixed:
 - Items move to overflow too soon: this happens because the space
   allocated to separate panel and non-panel items is equal to a button
   width. Changed it to a regular spacing between buttons;
 - Extra spacing at the end of shelf items, before tray items;
 - Discrepancy between ideal bounds calculated for items moved to the
   overflow shelf and the overflow button (they should be the same such
   that minimization animation for items in the overflow shelf ends at
   the overflow button or bubble for panel items in the overflow shelf
   points to the overflow button);
 - Possibility of panel app icons overlapping with overflow button;
 - Extra space before overflow button when it is the only item shown on
   the shelf (which happens when the launcher button is moved to the
   overflow shelf).

With these changes, showing system update icon is not necessary anymore
for some tests to pass. The hack is removed and the disabled test is
re-enabled (see  https://crbug.com/619344  and https://crbug.com/625671).

BUG=634128, 619344 ,625671
TEST=manual

Review-Url: https://codereview.chromium.org/2384103003
Cr-Commit-Position: refs/heads/master@{#433792}

[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/common/shelf/shelf_view.cc
[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/common/test/test_system_tray_delegate.cc
[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/common/test/test_system_tray_delegate.h
[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/test/ash_test_helper.cc
[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/888d1c0b366685696e5f4c73775575cc61bdab87/ash/wm/panels/panel_window_resizer_unittest.cc

Comment 6 by moh...@chromium.org, Nov 22 2016

Status: Fixed (was: Available)

Comment 7 by moh...@chromium.org, Nov 22 2016

Cc: jamescook@chromium.org
Owner: moh...@chromium.org
Labels: M-57

Comment 9 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 10 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment