New issue
Advanced search Search tips

Issue 923049 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Picture-in-picture window loses its control on click when Mash is enabled

Project Member Reported by mukai@chromium.org, Jan 17 (5 days ago)

Issue description

Chrome Version: ToT
OS: ChromeOS

What steps will reproduce the problem?
(1) run chrome with --enable-features=SingleProcessMash
(2) open youtube.com, open a video, right-click twice, select 'picture-in-picture'
(3) move the mouse cursor over the PIP window; controls (like play/stop button) appear
(4) click the play/stop button

What is the expected result?
- toggle the playing state
- the controls keep visible


What happens instead?
- toggle the playing state
- the controls disappear


Please use labels and text to provide additional information.

If this is a regression (i.e., worked before), please consider using
the bisect tool (https://www.chromium.org/developers/bisect-builds-py)
to help us identify the root cause and more rapidly triage the issue.

For graphics-related bugs, please copy/paste the contents of the
about://gpu page at the end of this report.


 

Comment 1 by mukai@chromium.org, Jan 17 (5 days ago)

As I debugged slightly, it seems the window server fails to activate the PIP window for some reasons, which reverts the activation requests, causes blur event, and so they disappear.

Comment 2 by osh...@chromium.org, Jan 17 (5 days ago)

PIP windows are not activatable by default. It can be activated only through accessibility.

Comment 3 by mukai@chromium.org, Jan 17 (5 days ago)

It's an always-on-top window, but somehow Mash codebase tries to activate it on a mouse pressed event. We'll have to fix DesktopWindowTreeHostMus or something.

Comment 4 by osh...@chromium.org, Jan 17 (5 days ago)

can you revert this change back (only overlay_window_view.cc) and see if it helps on mash?

https://chromium-review.googlesource.com/c/chromium/src/+/1357819/12/chrome/browser/ui/views/overlay/overlay_window_views.cc

Comment 5 by mukai@chromium.org, Jan 17 (5 days ago)

no, just set_can_activate(false) doesn't help here
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit 2bcce6449fcc55bc3ac94198c6d56f2d69063a69
Author: Jun Mukai <mukai@chromium.org>
Date: Fri Jan 18 17:34:52 2019

Set always-on-top widgets non-activatable in Mash

Since AshFocusRules consider such windows non-activatable, this
should be aligned in the window service client. Otherwise
client-side focus -> window-service attempts to focus ->
fail -> window blurred happen.

Bug:  923049 
Test: views_unittests
Change-Id: I7d0409b31c9c9188ab4d50f16451f4136bd41e76
Reviewed-on: https://chromium-review.googlesource.com/c/1419221
Auto-Submit: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624182}
[modify] https://crrev.com/2bcce6449fcc55bc3ac94198c6d56f2d69063a69/ui/views/mus/desktop_window_tree_host_mus_unittest.cc
[modify] https://crrev.com/2bcce6449fcc55bc3ac94198c6d56f2d69063a69/ui/views/widget/desktop_aura/desktop_focus_rules.cc

Comment 7 by mukai@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Started)

Comment 8 by osh...@chromium.org, Jan 18 (4 days ago)

Status: Assigned (was: Fixed)
Please see my comment on CL.

Comment 9 by mukai@chromium.org, Jan 18 (4 days ago)

Ugh, sorry. reverting the CL.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit 61720bf7f5d2982e6492005ca9a3e3c258517586
Author: Jun Mukai <mukai@chromium.org>
Date: Fri Jan 18 18:50:34 2019

Revert "Set always-on-top widgets non-activatable in Mash"

This reverts commit 2bcce6449fcc55bc3ac94198c6d56f2d69063a69.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Set always-on-top widgets non-activatable in Mash
> 
> Since AshFocusRules consider such windows non-activatable, this
> should be aligned in the window service client. Otherwise
> client-side focus -> window-service attempts to focus ->
> fail -> window blurred happen.
> 
> Bug:  923049 
> Test: views_unittests
> Change-Id: I7d0409b31c9c9188ab4d50f16451f4136bd41e76
> Reviewed-on: https://chromium-review.googlesource.com/c/1419221
> Auto-Submit: Jun Mukai <mukai@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#624182}

TBR=mukai@chromium.org,sky@chromium.org

Change-Id: Icf52e560e5b4ad0200e039ba8e365fd36b99c0d0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  923049 
Reviewed-on: https://chromium-review.googlesource.com/c/1422581
Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624214}
[modify] https://crrev.com/61720bf7f5d2982e6492005ca9a3e3c258517586/ui/views/mus/desktop_window_tree_host_mus_unittest.cc
[modify] https://crrev.com/61720bf7f5d2982e6492005ca9a3e3c258517586/ui/views/widget/desktop_aura/desktop_focus_rules.cc

Project Member

Comment 11 by bugdroid, Today (6 hours ago)

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

commit 422db43aa6c2a11af730f8b3b104f3982c24ef0f
Author: Jun Mukai <mukai@chromium.org>
Date: Tue Jan 22 23:54:47 2019

Set ACTIVATABLE_NO for PIP windows in ChromeOS.

So far there's no way to share 'can_activate' settings between
Ash and the browser, so setting this flag explicitly is the easiest
way to achieve the desired behavior.

Bug:  923049 
Test: manually
Change-Id: I4e7047aadc7b091278ffb85cd3adaff956e62aa7
Reviewed-on: https://chromium-review.googlesource.com/c/1423397
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624990}
[modify] https://crrev.com/422db43aa6c2a11af730f8b3b104f3982c24ef0f/chrome/browser/ui/views/overlay/overlay_window_views.cc

Comment 12 by mukai@chromium.org, Today (6 hours ago)

Status: Fixed (was: Assigned)

Sign in to add a comment