New issue
Advanced search Search tips

Issue 897875 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Mash: Window activation isn't switching properly when hiding

Project Member Reported by mukai@chromium.org, Oct 22

Issue description

What steps will reproduce the problem?
(1) run with --enable-features=SingleProcessMash
(2) ctrl-n to create a new browser window (now two windows exist)
(3) hit the minimize button on a currently focused window

What is the expected result?
- the window gets minimized
- the other window gets activated and gains the focus

What happens instead?
- the window gets minimized, but the other window remains deactivated



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.


 
This would be related to the failures of  issue 896080 .
Here's flow of events:
- first, window's property gets updated (normal -> minimized), resulting a mojo method (A)
- window gets hidden; SetFocus(nullptr) and VisibilityChange happens, resulting two mojo methods. Let's call SetFocus (B), and VisibilityChange (C).
- Server gets the window state change (A), picks up a new focus. It so calls OnWindowFocused (D) and acks to (A).
- Then server gets SetFocus(nullptr) (B), does nothing but acks.
- Then server processes the visibility change (C)
- client gets (D) mojo method. Since (B) is not yet replied, this focus is applied to the in-flight change.
- client gets (B)'s ack, which is successful. So the in-flight change is discarded.
- (and so (A)'s ack and (C)'s ack are also processed, but they are fine)

It seems we don't have to send SetFocus (B) since it's anyways ignored by the server and it is meaningful only within the client-side.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 23

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

commit 1ce3f686b09db96b980451b1fdb1bfb0987885ed
Author: Jun Mukai <mukai@chromium.org>
Date: Tue Oct 23 20:53:53 2018

Do not send SetFocus(nullptr) on focus reset

When window is hiding, there are several mojo messages are
exchanged and then Ash will pick up the new focused window and
notifies to the client through OnWindowFocused(). If the
OnWindowFocused() happens between SetFocus(nullptr) and its
response, the client will ignore the new focus.

Since SetFocus(nullptr) is anyways ignored on the server,
this means the client shouldn't send SetFocus(nullptr). It should
reset the focus within the client, but it shouldn't propagate
to the server.

BUG= 897875 
TEST=manually checked, the edits on the tests clarify the case too

Change-Id: I33439664354ee8fad646fe4b96682e6b726a794c
Reviewed-on: https://chromium-review.googlesource.com/c/1296680
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602081}
[modify] https://crrev.com/1ce3f686b09db96b980451b1fdb1bfb0987885ed/services/ws/focus_handler.cc
[modify] https://crrev.com/1ce3f686b09db96b980451b1fdb1bfb0987885ed/services/ws/focus_handler_unittest.cc
[modify] https://crrev.com/1ce3f686b09db96b980451b1fdb1bfb0987885ed/ui/aura/mus/focus_synchronizer.cc
[modify] https://crrev.com/1ce3f686b09db96b980451b1fdb1bfb0987885ed/ui/aura/mus/window_tree_client_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment