Fix Restore after Minimize on X11.

Fix Restore after Minimize on X11.

BUG=723986
Review-Url: https://codereview.chromium.org/2891893002
Cr-Commit-Position: refs/heads/master@{#474211}
Committed: https://chromium.googlesource.com/chromium/src/+/dd8019cfb439f7ebdc135820179f1cfc5c810b45

[alshabalin, 2017-05-18 07:28:52]

Description was changed from

==========
Fix Restore after Minimize on X11.

BUG=723986
==========

to

==========
Fix Restore after Minimize on X11.

BUG=723986
==========

[alshabalin, 2017-05-18 07:28:52]

alshabalin@yandex-team.ru changed reviewers:
+ erg@chromium.org, msw@chromium.org

[Elliot Glaysher, 2017-05-18 17:12:21]

lgtm

[msw, 2017-05-18 18:37:44]

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/desktop_aur...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/desktop_aur...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:571: if
(!IsVisible() || !window_mapped_in_server_)
I hope Elliot understands this aspect of the CL, I don't!

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/widget_inte...
File ui/views/widget/widget_interactive_uitest.cc (right):

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/widget_inte...
ui/views/widget/widget_interactive_uitest.cc:237: PropertyWaiter(const
base::Callback<bool(void)>& property,
nit: rename |callback| here and in class and comment?

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/widget_inte...
ui/views/widget/widget_interactive_uitest.cc:1260: PropertyWaiter
minimize_waiter(
Could a single base::RunLoop().RunUntilIdle(); handle these operations?
It would be nice to avoid adding this one-off PropertyWaiter if possible.

[alshabalin, 2017-05-23 06:48:18]

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/desktop_aur...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/desktop_aur...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:571: if
(!IsVisible() || !window_mapped_in_server_)
On 2017/05/18 18:37:44, msw wrote:
> I hope Elliot understands this aspect of the CL, I don't!

The problem was: during minimization some WMs (e.g. openbox) unmap the window
(which sets window_mapped_in_server_ to false). Now, when we call
Widget::Restore, we need to map the window back but at that moment
Widget::IsVisible() is true (because window_mapped_in_client_ is true).

I decided to go for this solution instead of modifying window_mapped_in_client_
during minimization, because asking to map window seems to makes sense when
either of window_mapped_in_* is false.

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/widget_inte...
File ui/views/widget/widget_interactive_uitest.cc (right):

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/widget_inte...
ui/views/widget/widget_interactive_uitest.cc:237: PropertyWaiter(const
base::Callback<bool(void)>& property,
On 2017/05/18 18:37:44, msw wrote:
> nit: rename |callback| here and in class and comment?

Done.

https://codereview.chromium.org/2891893002/diff/1/ui/views/widget/widget_inte...
ui/views/widget/widget_interactive_uitest.cc:1260: PropertyWaiter
minimize_waiter(
On 2017/05/18 18:37:44, msw wrote:
> Could a single base::RunLoop().RunUntilIdle(); handle these operations?
> It would be nice to avoid adding this one-off PropertyWaiter if possible.

I tried that initially and it seemed to work, but decided against it because it
might flake: there's no guarantee when WM will honor minimization request.

[msw, 2017-05-23 16:50:46]

lgtm

[alshabalin, 2017-05-24 07:20:32]

The CQ bit was checked by alshabalin@yandex-team.ru

[alshabalin, 2017-05-24 07:20:32]

The patchset sent to the CQ was uploaded after l-g-t-m from erg@chromium.org
Link to the patchset: https://codereview.chromium.org/2891893002/#ps20001
(title: "Fix review comments")

[commit-bot: I haz the power, 2017-05-24 07:20:56]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 08:22:38]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495610432434190,
"parent_rev": "4f81f3151ff0d7e1c04a234fa0c4ba4d4faed1a8", "commit_rev":
"dd8019cfb439f7ebdc135820179f1cfc5c810b45"}

[commit-bot: I haz the power, 2017-05-24 08:22:48]

Description was changed from

==========
Fix Restore after Minimize on X11.

BUG=723986
==========

to

==========
Fix Restore after Minimize on X11.

BUG=723986

Review-Url: https://codereview.chromium.org/2891893002
Cr-Commit-Position: refs/heads/master@{#474211}
Committed:
https://chromium.googlesource.com/chromium/src/+/dd8019cfb439f7ebdc135820179f...
==========

[commit-bot: I haz the power, 2017-05-24 08:22:49]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/dd8019cfb439f7ebdc135820179f...