New issue
Advanced search Search tips

Issue 781925 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 678705



Sign in to add a comment

mash: Remove ash access from chrome/browser/ui/ash

Project Member Reported by jamescook@chromium.org, Nov 6 2017

Issue description

Replace with mojo apis. See ash/README.md and go/mustash.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 7 2017

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

commit 14b4134077b54e0278bb651e008fd069a6c539a7
Author: James Cook <jamescook@chromium.org>
Date: Tue Nov 07 21:03:14 2017

cros: Adapt ShelfBrowserTest to work with --mash

* Add a mojo shelf test API
* Convert 2 tests to use it

Bug: 781925,  678687 
Test: browser_tests --mash
Change-Id: If75111a9795bc93eefb69bc2d80401d1fb2b6d65
Reviewed-on: https://chromium-review.googlesource.com/755344
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514582}
[modify] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/BUILD.gn
[modify] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/mojo_test_interface_factory.cc
[modify] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/mus/manifest.json
[modify] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/public/interfaces/shelf_test_api.mojom
[add] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/shelf/shelf_test_api.cc
[add] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/ash/shelf/shelf_test_api.h
[modify] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/chrome/browser/ui/ash/shelf_browsertest.cc
[modify] https://crrev.com/14b4134077b54e0278bb651e008fd069a6c539a7/testing/buildbot/filters/mash.browser_tests.filter

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8 2017

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

commit 9716183168af9fd41917aca3ce613b9801b670c3
Author: James Cook <jamescook@chromium.org>
Date: Wed Nov 08 02:56:57 2017

Revert "cros: Adapt ShelfBrowserTest to work with --mash"

This reverts commit 14b4134077b54e0278bb651e008fd069a6c539a7.

Reason for revert: Causes flaky failures in mash_browser_tests. I can repro the failures locally. Sample from bots:

https://chromium-swarm.appspot.com/task?id=39b1d0d34a750d10&refresh=10&show_raw=1&wide_logs=true output is:
    
    ../../chrome/browser/ui/ash/shelf_browsertest.cc:76: Failure
    Value of: has_overlapping_window
      Actual: false
    Expected: true
    [500:500:1107/162023.401861:INFO:chrome_cryptauth_service.cc(230)] Refresh token not yet available; waiting before starting CryptAuth managers.
    [534:534:1107/162023.407440:ERROR:shell_delegate_mus.cc(75)] Not implemented reached in virtual void ash::ShellDelegateMus::PreShutdown()
    [  FAILED  ] ShelfBrowserTest.StatusBubble, where TypeParam =  and GetParam() =  (962 ms)

Original change's description:
> cros: Adapt ShelfBrowserTest to work with --mash
> 
> * Add a mojo shelf test API
> * Convert 2 tests to use it
> 
> Bug: 781925,  678687 
> Test: browser_tests --mash
> Change-Id: If75111a9795bc93eefb69bc2d80401d1fb2b6d65
> Reviewed-on: https://chromium-review.googlesource.com/755344
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Commit-Queue: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#514582}

TBR=jamescook@chromium.org,msw@chromium.org,tsepez@chromium.org

Change-Id: I6e4bea721b44cacb2cf53d9c8861e7d4889fb4de
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 781925,  678687 
Reviewed-on: https://chromium-review.googlesource.com/757792
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514722}
[modify] https://crrev.com/9716183168af9fd41917aca3ce613b9801b670c3/ash/BUILD.gn
[modify] https://crrev.com/9716183168af9fd41917aca3ce613b9801b670c3/ash/mojo_test_interface_factory.cc
[modify] https://crrev.com/9716183168af9fd41917aca3ce613b9801b670c3/ash/mus/manifest.json
[modify] https://crrev.com/9716183168af9fd41917aca3ce613b9801b670c3/ash/public/interfaces/BUILD.gn
[delete] https://crrev.com/e704e6785d9a47fe89e5030b9a3d7340360d5709/ash/public/interfaces/shelf_test_api.mojom
[delete] https://crrev.com/e704e6785d9a47fe89e5030b9a3d7340360d5709/ash/shelf/shelf_test_api.cc
[delete] https://crrev.com/e704e6785d9a47fe89e5030b9a3d7340360d5709/ash/shelf/shelf_test_api.h
[modify] https://crrev.com/9716183168af9fd41917aca3ce613b9801b670c3/chrome/browser/ui/ash/shelf_browsertest.cc
[modify] https://crrev.com/9716183168af9fd41917aca3ce613b9801b670c3/testing/buildbot/filters/mash.browser_tests.filter

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 9 2017

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

commit 8f09721fadf98c35df07c2db04634135a7c72ea2
Author: James Cook <jamescook@chromium.org>
Date: Thu Nov 09 00:18:54 2017

Reland: cros: Adapt ShelfBrowserTest to work with --mash

The original CL caused mash_browser_test flake because the test was not
waiting for in-flight window bounds/visibility changes to apply, so
there was a race between browser, window server and ash.

Original CL description:
* Add a mojo shelf test API
* Convert 2 tests to use it

Bug: 781925,  678687 
Test: browser_tests --mash
Change-Id: Icfe23d4c8c3bcd84bff8dc46d2c9680cc656ce50
Reviewed-on: https://chromium-review.googlesource.com/755344
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514582}

TBR=tsepez@chromium.org

Change-Id: Icfe23d4c8c3bcd84bff8dc46d2c9680cc656ce50
Reviewed-on: https://chromium-review.googlesource.com/759034
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515028}
[modify] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/BUILD.gn
[modify] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/mojo_test_interface_factory.cc
[modify] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/mus/manifest.json
[modify] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/public/interfaces/shelf_test_api.mojom
[add] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/shelf/shelf_test_api.cc
[add] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/ash/shelf/shelf_test_api.h
[modify] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/chrome/browser/ui/ash/shelf_browsertest.cc
[modify] https://crrev.com/8f09721fadf98c35df07c2db04634135a7c72ea2/testing/buildbot/filters/mash.browser_tests.filter

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 9 2017

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

commit 928f23c3c314be28cec7f3d9a5658218ffefd334
Author: James Cook <jamescook@chromium.org>
Date: Thu Nov 09 22:45:50 2017

cros: Clean up TrayAccessibilityTest

This browser test has code that simulates login in a way that requires
test-only injection of profiles and PrefServices into various a11y
classes.  Instead of doing that, split out the code that tests the
login screen into an OobeBaseTest.

This allows some test support code to be removed from
ash::AccessibilityController, reduces a couple of includes of ash
headers from chrome, and is a step toward mash support for this test.

Bug: 781925
Test: browser_tests
Change-Id: Ib710ae75c4db3801e067c74d7ae354cff2eff857
Reviewed-on: https://chromium-review.googlesource.com/761409
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515330}
[modify] https://crrev.com/928f23c3c314be28cec7f3d9a5658218ffefd334/ash/accessibility/accessibility_controller.cc
[modify] https://crrev.com/928f23c3c314be28cec7f3d9a5658218ffefd334/ash/accessibility/accessibility_controller.h
[modify] https://crrev.com/928f23c3c314be28cec7f3d9a5658218ffefd334/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc

Components: -Internals>MUS Internals>Services>WindowService
Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash

Sign in to add a comment