Could not take window screenshot of shortcut app window |
|||||||||
Issue descriptionChrome Version: Version 69.0.3486.0 (Official Build) dev (64-bit) OS: 10866.1.0 (Official Build) dev-channel eve ctrl+alt+f4 to take window screenshot of shortcut app window. Shortcut app window could not be correctly selected and it fails on saving as well.
,
Oct 6
looks like mojo app issue. mukai-san, can you take a look?
,
Oct 8
msw, could you take a look? Or rcui?
,
Oct 8
Is this with Mash, SingleProcessMash, or neither? Screenshots don't work in Mash yet (crbug.com/557397).
,
Oct 8
I think neither -- but the keyboard shortcut viewer is still a Mojo process and that could be affected?
,
Oct 8
#5, Yes, that's my guess.
,
Oct 8
To clarify, Ctrl-F4 (full screenshot) and Ctrl-Shift-F4 (region selected screenshot) are working, right? If so, this would be essentially a dupe for issue 557397. Reassigning to rcui. Ryan, feel free to mark as duplicated.
,
Oct 8
Some observations (via Chrome 71.0.3567.0 on dev channel): 1. Ctrl-F4 (full screenshot) works, with and without shortcut viewer. 2. Ctrl-Shift-F4 (region selection) works, with and without shortcut viewer. 3. Ctrl-Alt-F4 (window selection) works fine without shortcut viewer. With the shortcut viewer open, it's very difficult to select the shortcut viewer window, and the code keeps highlighting a 'ghost' window in the top-left corner. See pic attached. When I click on the ghost window, I get a "Failed to save screenshot" error. As this happens without Mash or SingleProcessMash enabled, it's not a dupe of issue 557397. Re-assigning to jamescook@ for thoughts here.
,
Oct 8
,
Oct 8
Ryan, would you mind looking a bit closer? How does the window selection screenshot codepath work? It's not clear what is actually broken when handling mojo app windows like the ksv. If you can point out some relevant codepaths, I might be able to investigate further.
,
Oct 8
https://cs.chromium.org/chromium/src/ui/snapshot/snapshot.cc?rcl=f52d6249596258ba819c6f6a3f081594711ff61f&l=52 |source_rect| is relative to the |window|, but it seems to be used as screen coords for mojo app.
,
Oct 8
sorry, correction. > but it seems to be used as screen coords for mojo app. it seems that the screen coords rect is passed for mojo app. https://cs.chromium.org/chromium/src/ui/snapshot/snapshot_aura.cc?rcl=4e72f72afe4b4c78a7e210b68992ebc72de0a526&l=31 This API may also have an issue if the |window| is mojo app's root because it gives error even if I move the window to top,left.
,
Oct 9
Digging into this a bit more. Re: oshima@'s comment in #12, I believe the coordinate rect is simply the Size of the aura::Window being captured[1]. I don't know enough yet about how mojo apps handle windows to provide more details. Will look at dumping the window hierarchy and see if that provides more clues. [1] https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/chrome_screenshot_grabber.cc?rcl=71ca70dd801046e681ff7fc2a81a2f5c64dfa3bc&l=455
,
Oct 9
I'm not familiar with that code (nor compositing/layers), but clicking through took me here: https://cs.chromium.org/chromium/src/ui/snapshot/snapshot_aura.cc?rcl=d8f40759b05fdf720be7ca17537dd722505bd81d&l=75 Perhaps the window's layer (as seen by Ash/Chrome) is not valid for screenshotting in that way? Do the other screenshot codepaths use individual window layers or some other composited surface?
,
Oct 9
#14: other screenshot uses root windows (of Ash) to capture the screenshot. The bounds wierdness and failure might mean that the 'window' in #13 could be a window in Browser and not in Ash?
,
Oct 9
The window in question is for the KSV app, which is running as a mojo app in its own process. We should solve this however we intend to support per-window screenshots in Mash generally.
,
Oct 9
Okay, I guess the primary reason wasn't Mash-not-supporting-screenshot. It looks like due to a change on window structure caused by the keyboard-shortcut viewer. Previously it was just a window (with views::Widget) directly under DefaultContainer, and the current screenshot code probably assumes the window is toplevel. But now KSV consists of two windows in Ash, one is KeyboardShortcutWidget (i.e. window frame, which is toplevel) and the content window (drawn by a mojo app). If the mouse moves around the content area of KSV, the event target becomes the content window, and starts misbehaving. When the mouse cursor moves around the title bar of the KSV, the selector properly highlight the window and the screenshot can be taken successfully. Maybe we need to fix on Ash side. I can take a look.
,
Oct 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/965bb35d095c9cf3e31c12e43bdbe82cdff4da00 commit 965bb35d095c9cf3e31c12e43bdbe82cdff4da00 Author: Jun Mukai <mukai@chromium.org> Date: Fri Oct 12 22:57:47 2018 Skip non-toplevel child windows for screenshot window selector The Keyboard Shortcut Viewer hits this condition -- now it has a toplevel widget with child content window, and when the mouse pointer comes to the content window's area, it is unexpectedly selected, but the toplevel one should be selected. BUG= 863728 TEST=manually, and the new test case covers Change-Id: I9962114c6b14df412148cf9e6545f32fc5eaa845 Reviewed-on: https://chromium-review.googlesource.com/c/1272089 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Commit-Queue: Jun Mukai <mukai@chromium.org> Cr-Commit-Position: refs/heads/master@{#599395} [modify] https://crrev.com/965bb35d095c9cf3e31c12e43bdbe82cdff4da00/ash/utility/screenshot_controller.cc [modify] https://crrev.com/965bb35d095c9cf3e31c12e43bdbe82cdff4da00/ash/utility/screenshot_controller_unittest.cc [modify] https://crrev.com/965bb35d095c9cf3e31c12e43bdbe82cdff4da00/services/ws/window_service.cc [modify] https://crrev.com/965bb35d095c9cf3e31c12e43bdbe82cdff4da00/services/ws/window_service.h
,
Oct 12
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by wutao@chromium.org
, Oct 5