New issue
Advanced search Search tips

Issue 632111 link

Starred by 2 users

Issue metadata

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

Blocked on:
issue 557397

Blocking:
issue 612331



Sign in to add a comment

Convert ScreenShotDelegate to ash common types

Project Member Reported by sky@chromium.org, Jul 27 2016

Issue description

ScreenShotDelegate needs to be moved in terms of common types.
 

Comment 1 by sky@chromium.org, Jul 27 2016

Blocking: 612331

Comment 2 by sky@chromium.org, Aug 24 2016

Labels: Proj-Mustash-Mash
Components: MUS
Labels: Proj-Mustash
Components: Internals>MUS

Comment 6 by sky@chromium.org, Feb 17 2017

There are a number of tests that will be disabled because of this. When re-enabling be sure to search for all references to this bug.

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

Blockedon: 557397
Labels: -Pri-3 -mash -mash-wm Pri-2
Owner: jamescook@chromium.org
Status: Started (was: Untriaged)
"ash common types" isn't relevant anymore. We still need screenshot support for mash, but we can re-enable a bunch of tests then close this.

Project Member

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

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

commit 5e9d340366992d63fc03a2f814b218f349e5e551
Author: James Cook <jamescook@chromium.org>
Date: Wed Nov 01 00:20:12 2017

cros: Fix ash ScreenshotDelegate ownership and creation

Many ash_unittests fail under --mus and --mash because the
TestScreenshotDelegate is not created. This happens because of some
legacy AcceleratorControllerDelegate code that dates back ~18 months
to when we couldn't use aura::Window* in mash code. That isn't a
problem anymore.

* Move ownership of ScreenshotDelegate to ScreenshotController
* Create the delegate in ShellDelegate, similar to how we do other
  ash delegates
* Move screenshot accelerator code back into AcceleratorController
* Re-enable tests under mus and mash via filter files

Screenshots don't work yet under --mus and --mash due to lack of
graphics readback support and lack of a mojo interface, but we now
exercise more of the UI and accelerator handling code.

Bug: 557397,  632111 
Test: ash_unittests (--mus, --mash), manually take screenshots
Change-Id: I7dd1946ddc30bddb407c9dbf6095caf8efb4fd1f
Reviewed-on: https://chromium-review.googlesource.com/744295
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513018}
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/accelerators/accelerator_controller_delegate_classic.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/accelerators/accelerator_controller_delegate_classic.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/accelerators/accelerator_controller_unittest.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/mus/accelerators/accelerator_controller_delegate_mus.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/mus/bridge/shell_port_mash.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/shell.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/shell_delegate.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/system/palette/tools/capture_region_mode.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/system/palette/tools/capture_screen_action.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/system/palette/tools/screenshot_unittest.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/test/ash_test_base.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/test/ash_test_helper.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/test/ash_test_helper.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/test_shell_delegate.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/test_shell_delegate.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/utility/screenshot_controller.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/utility/screenshot_controller.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/ash/utility/screenshot_controller_unittest.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/DEPS
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/ash/ash_init.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/ash/chrome_screenshot_grabber.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/ash/chrome_screenshot_grabber.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/ash/chrome_screenshot_grabber_browsertest.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/chrome/browser/ui/browser_commands_chromeos.cc
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/testing/buildbot/filters/ash_unittests_mash.filter
[modify] https://crrev.com/5e9d340366992d63fc03a2f814b218f349e5e551/testing/buildbot/filters/ash_unittests_mus.filter

Project Member

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

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

commit d2c8246cebfbf01e4b6ecdc65ffd8d0189c20a69
Author: James Cook <jamescook@chromium.org>
Date: Wed Nov 01 15:20:19 2017

cros: Enable AcceleratorFilterTests under mash

These pass under ash_unittests --mash now that we support the
TestScreenshotDelegate.

Bug:  632111 
Test: ash_unittests --mash
Change-Id: Ia5b6dc816ba61554f57327fc0e94101f50e5bf10
Reviewed-on: https://chromium-review.googlesource.com/748316
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513131}
[modify] https://crrev.com/d2c8246cebfbf01e4b6ecdc65ffd8d0189c20a69/ash/accelerators/accelerator_filter_unittest.cc

Status: Fixed (was: Started)
Calling this done, since "wmcommon" types don't exist any more. We'll need an entirely different approach for screenshots under --mash. That's issue 557397.

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

Status: Archived (was: Fixed)

Comment 13 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Components: -Internals>MUS Internals>Services>WindowService
Components: -MUS

Sign in to add a comment