[ash-md] Does not cancel overview when focus shifts from text fitler

[ash-md] Does not cancel overview when focus shifts from text fitler

This fixes a crash when transferring focus from text filter to a context
menu causes overview mode to close which then causes focus to be
attempted to be set on a window that no longer exists (text filter).

BUG=616407
TEST=WindowSelectorTest.ShowTextFilterMenu

Committed: https://crrev.com/37b21c76cd431944d40c03a589798c09e1919dbd
Cr-Commit-Position: refs/heads/master@{#414115}

[varkha, 2016-08-24 04:14:29]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 04:14:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[varkha, 2016-08-24 04:17:01]

varkha@chromium.org changed reviewers:
+ tdanderson@chromium.org

[varkha, 2016-08-24 04:17:02]

tdanderson@, can you please take a look? There could be some edge cases but I
think this is safe. WDYT?

[commit-bot: I haz the power, 2016-08-24 04:58:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 04:58:50]

Dry run: This issue passed the CQ dry run.

[tdanderson, 2016-08-24 14:49:25]

A couple of questions below:

https://chromiumcodereview.appspot.com/2276853002/diff/1/ash/common/wm/overvi...
File ash/common/wm/overview/window_selector.cc (right):

https://chromiumcodereview.appspot.com/2276853002/diff/1/ash/common/wm/overvi...
ash/common/wm/overview/window_selector.cc:614: } else if
(showing_selection_widget_ &&
What if the string entered into the text filter widget does not match any window
titles - won't |showing_selection_widget_| be false in that case?

https://chromiumcodereview.appspot.com/2276853002/diff/1/ash/wm/overview/wind...
File ash/wm/overview/window_selector_unittest.cc (right):

https://chromiumcodereview.appspot.com/2276853002/diff/1/ash/wm/overview/wind...
ash/wm/overview/window_selector_unittest.cc:1951: // Open system bubble shifting
focus from the text filter.
So what happens if you do the following?

1) Enter overview
2) Type something into the text filter
3) Open the system menu
4) Click on the Settings row to open WebUI settings in a new window

[varkha, 2016-08-24 16:53:54]

https://codereview.chromium.org/2276853002/diff/1/ash/common/wm/overview/wind...
File ash/common/wm/overview/window_selector.cc (right):

https://codereview.chromium.org/2276853002/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector.cc:614: } else if
(showing_selection_widget_ &&
On 2016/08/24 14:49:25, tdanderson wrote:
> What if the string entered into the text filter widget does not match any
window
> titles - won't |showing_selection_widget_| be false in that case?

No. |showing_selection_widget_| is indicating visibility of the text filter
rather than "selection widget" (aka WindowGrid::selection_widget_|. Name reuse I
guess. I will rename the variable to reflect that.

https://codereview.chromium.org/2276853002/diff/1/ash/wm/overview/window_sele...
File ash/wm/overview/window_selector_unittest.cc (right):

https://codereview.chromium.org/2276853002/diff/1/ash/wm/overview/window_sele...
ash/wm/overview/window_selector_unittest.cc:1951: // Open system bubble shifting
focus from the text filter.
On 2016/08/24 14:49:25, tdanderson wrote:
> So what happens if you do the following?
> 
> 1) Enter overview
> 2) Type something into the text filter
> 3) Open the system menu
> 4) Click on the Settings row to open WebUI settings in a new window

Activation of Settings WebUI window in step 4 will close overview so WAI.

[tdanderson, 2016-08-24 17:43:52]

lgtm

https://codereview.chromium.org/2276853002/diff/1/ash/common/wm/overview/wind...
File ash/common/wm/overview/window_selector.cc (right):

https://codereview.chromium.org/2276853002/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector.cc:614: } else if
(showing_selection_widget_ &&
On 2016/08/24 16:53:54, varkha wrote:
> On 2016/08/24 14:49:25, tdanderson wrote:
> > What if the string entered into the text filter widget does not match any
> window
> > titles - won't |showing_selection_widget_| be false in that case?
> 
> No. |showing_selection_widget_| is indicating visibility of the text filter
> rather than "selection widget" (aka WindowGrid::selection_widget_|. Name reuse
I
> guess. I will rename the variable to reflect that.

Thanks.

https://codereview.chromium.org/2276853002/diff/1/ash/wm/overview/window_sele...
File ash/wm/overview/window_selector_unittest.cc (right):

https://codereview.chromium.org/2276853002/diff/1/ash/wm/overview/window_sele...
ash/wm/overview/window_selector_unittest.cc:1951: // Open system bubble shifting
focus from the text filter.
On 2016/08/24 16:53:54, varkha wrote:
> On 2016/08/24 14:49:25, tdanderson wrote:
> > So what happens if you do the following?
> > 
> > 1) Enter overview
> > 2) Type something into the text filter
> > 3) Open the system menu
> > 4) Click on the Settings row to open WebUI settings in a new window
> 
> Activation of Settings WebUI window in step 4 will close overview so WAI.

Acknowledged.

[varkha, 2016-08-24 18:06:14]

The CQ bit was checked by varkha@chromium.org

[commit-bot: I haz the power, 2016-08-24 18:06:58]

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, 2016-08-24 18:51:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-24 18:53:14]

Description was changed from

==========
[ash-md] Does not cancel overview when focus shifts from text fitler

This fixes a crash when transferring focus from text filter to a context
menu causes overview mode to close which then causes focus to be
attempted to be set on a window that no longer exists (text filter).

BUG=616407
TEST=WindowSelectorTest.ShowTextFilterMenu
==========

to

==========
[ash-md] Does not cancel overview when focus shifts from text fitler

This fixes a crash when transferring focus from text filter to a context
menu causes overview mode to close which then causes focus to be
attempted to be set on a window that no longer exists (text filter).

BUG=616407
TEST=WindowSelectorTest.ShowTextFilterMenu

Committed: https://crrev.com/37b21c76cd431944d40c03a589798c09e1919dbd
Cr-Commit-Position: refs/heads/master@{#414115}
==========

[commit-bot: I haz the power, 2016-08-24 18:53:15]

Patchset 2 (id:??) landed as
https://crrev.com/37b21c76cd431944d40c03a589798c09e1919dbd
Cr-Commit-Position: refs/heads/master@{#414115}