New issue
Advanced search Search tips

Issue 698887 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 693114



Sign in to add a comment

PanelLayoutManagerTest failuires in mash

Project Member Reported by sky@chromium.org, Mar 6 2017

Issue description

Specifically the icon bounds look wrong.
 

Comment 1 by sky@chromium.org, Mar 7 2017

Blocking: 693114
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 7 2017

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

commit ea37af717f1569dd93282e09b6fb49ff22b771f8
Author: sky <sky@chromium.org>
Date: Tue Mar 07 22:59:41 2017

Last sets of tests that can move to common_unittests

This way they run in both mash_unittests and ash_unittests.
Interesting things of note:
. Had to remove DCHECKs for GetRootWindowForDisplayId() as some tests
  call with invalid.
. Had to rename class in mus named WindowManagerTest as it conflicts
  with test with same name in ash.

BUG= 622486 , 631103 , 637853 , 648733 ,695556, 696028 ,698091, 698129 , 698878 , 698887 ,698888,698892,698894, 698895 , 698914 ,699172,699175, 693114 
TEST=test changes
R=msw@chromium.org

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

[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/BUILD.gn
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/mus/bridge/wm_shell_mus.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/mus/window_manager_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/test/ash_test_base.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/system_modal_container_layout_manager_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/window_cycle_controller_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/window_manager_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/window_modality_controller_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/window_state_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/window_util_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/workspace/workspace_event_handler_unittest.cc
[modify] https://crrev.com/ea37af717f1569dd93282e09b6fb49ff22b771f8/ash/wm/workspace/workspace_window_resizer_unittest.cc

Comment 3 by msw@chromium.org, Apr 14 2017

Cc: msw@chromium.org
Summary: PanelLayoutManagerTest failuires in mash (was: PanelLayoutManagerTest.MultiplePanelCallout fails in mash)
PanelLayoutManagerTest.MultiplePanelStackingVertical and PanelLayoutManagerTest.FanWindows also fail a few expectations with my latest change. It's not clear what's going wrong, so I may disable [portions of] those tests in mash and investigate in a followup: See https://codereview.chromium.org/2791463002
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 18 2017

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

commit 448103a0613cc1d0170c2fddf815c5d39ba411f7
Author: msw <msw@chromium.org>
Date: Tue Apr 18 17:24:54 2017

mash: Prerequisites for removing ShelfDelegate.

This CL splits out some optionally prerequisite work from:
  https://codereview.chromium.org/2791463002/
Most changes are test-only, and shouldn't affect production.

Nix unused CreatePanel function in accelerator_controller_unittest.cc
Nix UserMetricsRecorderTest::CreateTestWindow, add ShelfModel items.
Nix unnecessary ShelfTooltipManagerTest and ShelfTest item delegates.

Make ShelfApplicationMenuModel work with a null item delegate.
(supports a simplification in the test file)

Make AshTestBase set the shelf item type property for test panels.
(uses ShelfWindowWatcher for test panels, similar to production)

Cleanup shelf_view_unittest.cc:
-Base classes on ash::ShelfItemDelegate, not TestShelfItemDelegate.
-Track the count of selections, rather than a bool & Reset().
-Add ShelfModel items directly instead of making windows for items.
-Nix TestShelfDelegateForShelfView, rely on TestShelfDelegate instead.
-Refactor helper functions for adding shelf items.

Make TestShelfDelegate directly manipulate ShelfModel items.
(behaves much more like CLC, the production ShelfDelegate impl)

Move ShelfInitializer from TestShelfDelegate to TestShellDelegate.
(TestShelfDelegate will be removed, but tests still need shelf init)

Exclude a few expectations for mash in PanelLayoutManagerTest.*

Refine PanelWindowResizerTest comments and expectations.
-Expect panel ordering like production (added right to left).
-Skip testing a panel as a transient child of a panel (odd, broken).

BUG= 557406 , 698887 
TEST=No Chrome OS shelf behavior changes. Tests pass.
R=jamescook@chomium.org
TBR=yusukes@chromium.org

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

[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/accelerators/accelerator_controller_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/metrics/user_metrics_recorder_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/shelf/shelf_application_menu_model.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/shelf/shelf_application_menu_model_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/shelf/shelf_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/shelf/wm_shelf.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/test/ash_test_base.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/test/test_shelf_delegate.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/test/test_shelf_delegate.h
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/test/test_shell_delegate.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/test/test_shell_delegate.h
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/wm/overview/window_selector_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/wm/window_cycle_controller_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/ash/wm/workspace_controller_unittest.cc
[modify] https://crrev.com/448103a0613cc1d0170c2fddf815c5d39ba411f7/chrome/browser/chromeos/arc/arc_session_manager.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 20 2017

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

commit 5138f3dd861b75ce64e9186b126c5fa8774a03e7
Author: msw <msw@chromium.org>
Date: Thu Apr 20 00:22:07 2017

mash: Remove ShelfDelegate; move functions to ShelfModel.

Remove ShelfDelegate, subclasses, accessors, etc.
Move ShelfDelegate responsibilities to ash::ShelfModel, add tests.

Move ChromeLauncherControllerImpl ownership to ChromeShellDelegate.
Add lifetime management functions: ShelfInit() and ShelfShutdown().
Apply similar ownership changes in tests; expose an AshTestHelper.

Remove ShelfDelegate responsibilities from ChromeLauncherController:
* Apply chrome-specific item info/delegates in ShelfItemAdded.
  - Support ShelfModel::SetItem calls amid ShelfItemAdded in ShelfView.
  - Register some item delegates early to avoid a default one.
* Cleanup chrome-side item info in ShelfItemRemoved.

TODO: Make ChromeLauncherControllerImpl lifetime more flexible.
TODO: Move functions to (or add accessors on) ash::[mojom::]ShelfController.

BUG= 557406 , 698887 
TEST=No Chrome OS shelf behavior changes.
R=jamescook@chromium.org,sky@chromium.org

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

[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/BUILD.gn
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/mus/BUILD.gn
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/mus/shelf_delegate_mus.cc
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/mus/shelf_delegate_mus.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/root_window_controller.cc
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/shelf/shelf_delegate.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shelf/shelf_model.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shelf/shelf_model.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shelf/shelf_model_unittest.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shelf/shelf_view.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shelf/shelf_view.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shelf/shelf_widget.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shell.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shell.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/shell_delegate.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/BUILD.gn
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/shelf_view_test_api.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/shelf_view_test_api.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/shell_test_api.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/shell_test_api.h
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/test/test_shelf_delegate.cc
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/test/test_shelf_delegate.h
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/test/test_shelf_item_delegate.cc
[delete] https://crrev.com/bd1281da74251c1116fad952dc794a107e5ba1e3/ash/test/test_shelf_item_delegate.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/test_shell_delegate.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/ash/test/test_shell_delegate.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/app_list/app_list_controller_ash.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/arc_app_launcher_browsertest.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/extension_app_window_launcher_controller.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/browser/ui/views/apps/app_info_dialog/app_info_footer_panel.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/test/base/browser_with_test_window_test.cc
[modify] https://crrev.com/5138f3dd861b75ce64e9186b126c5fa8774a03e7/chrome/test/base/browser_with_test_window_test.h

Components: Tests>Disabled
Labels: Test-Disabled
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 7 2018

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

commit 67827ab7ae7a329bcb7aa10fab37154f58363db3
Author: Mike Wasserman <msw@chromium.org>
Date: Wed Feb 07 00:29:54 2018

Enable shelf and panel ash_unittests in mash

These tests appear to pass as-is on ToT @ #534716.

Bug:  695563 ,  698887 ,  775177 
Test: Automated
Change-Id: I6d3979aec6738e4d14c2119faa3488d0ee1c596e
Reviewed-on: https://chromium-review.googlesource.com/905687
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534850}
[modify] https://crrev.com/67827ab7ae7a329bcb7aa10fab37154f58363db3/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/67827ab7ae7a329bcb7aa10fab37154f58363db3/ash/wm/panels/panel_layout_manager_unittest.cc

Comment 9 by msw@chromium.org, Feb 7 2018

Status: Fixed (was: Untriaged)

Sign in to add a comment