Remove ShouldBlockShelfAutoHide().

Remove ShouldBlockShelfAutoHide().

Instead, don't auto-hide the shelf if any TrayBubbleView is open.

I considered making this check if any TrayBackgroundView is active, but
some like the virtual keyboard can be active without wanting to block
the shelf from auto hiding.

BUG=686341
Review-Url: https://codereview.chromium.org/2831233003
Cr-Commit-Position: refs/heads/master@{#467342}
Committed: https://chromium.googlesource.com/chromium/src/+/6806c795184e4012764bf389f63f95c4e266d382

[Evan Stade, 2017-04-20 22:52:06]

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

[commit-bot: I haz the power, 2017-04-20 22:52:24]

Dry run: 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-04-20 22:55:10]

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

[commit-bot: I haz the power, 2017-04-20 22:55:11]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Evan Stade, 2017-04-21 01:08:00]

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

[commit-bot: I haz the power, 2017-04-21 01:08:24]

Dry run: 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-04-21 02:02:42]

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

[commit-bot: I haz the power, 2017-04-21 02:02:43]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2017-04-21 18:14:12]

estade@chromium.org changed reviewers:
+ sky@chromium.org

[Evan Stade, 2017-04-21 18:14:13]

sky, ptal.

This once again raises the issue that TrayBubbleView is probably located in the
wrong directory, only I don't know where it should be since it's used by both
ash/ and ui/message_center/.

[sky, 2017-04-21 20:43:50]

Do existing tests cover this?

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
File ui/views/bubble/tray_bubble_view.cc (right):

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
ui/views/bubble/tray_bubble_view.cc:248: ++g_current_tray_bubble_view_count_;
Is there a reason you like this here rather than the constructor? Is it because
you only care about actual showing bubbles?

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
File ui/views/bubble/tray_bubble_view.h (right):

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
ui/views/bubble/tray_bubble_view.h:90: static bool IsATrayBubbleOpen();
This should be after the destructor.

[Evan Stade, 2017-04-22 00:10:25]

On 2017/04/21 20:43:50, sky wrote:
> Do existing tests cover this?

I thought so, but I guess the tests were just running into some DCHECKs that I
have since removed. So I extended an ime menu tray unit test.

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
File ui/views/bubble/tray_bubble_view.cc (right):

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
ui/views/bubble/tray_bubble_view.cc:248: ++g_current_tray_bubble_view_count_;
On 2017/04/21 20:43:49, sky wrote:
> Is there a reason you like this here rather than the constructor? Is it
because
> you only care about actual showing bubbles?

yes. In fact the decrement needs to come earlier, in OnWidgetClosing, so I've
moved it there (this created a subtle bug where the shelf wouldn't hide right
away but rather waited until you moved the mouse by a pixel).

This also exposed a bug which I've addressed here:
https://codereview.chromium.org/2834943002/#

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
File ui/views/bubble/tray_bubble_view.h (right):

https://codereview.chromium.org/2831233003/diff/20001/ui/views/bubble/tray_bu...
ui/views/bubble/tray_bubble_view.h:90: static bool IsATrayBubbleOpen();
On 2017/04/21 20:43:50, sky wrote:
> This should be after the destructor.

Done.

[sky, 2017-04-24 15:32:40]

LGTM

https://codereview.chromium.org/2831233003/diff/60001/ui/views/bubble/tray_bu...
File ui/views/bubble/tray_bubble_view.cc (right):

https://codereview.chromium.org/2831233003/diff/60001/ui/views/bubble/tray_bu...
ui/views/bubble/tray_bubble_view.cc:54: int g_current_tray_bubble_view_count_ =
0;
optional: name this g_current_tray_showing_count (or something to indicate it
corresponds to showing bubbles).

[Evan Stade, 2017-04-26 15:22:25]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2017-04-26 15:22:26]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2831233003/#ps80001
(title: "rename and rebase")

[commit-bot: I haz the power, 2017-04-26 15:22:49]

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-04-26 16:27:28]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493220145858250,
"parent_rev": "aa935d4a5d6e57bd07fd6eb9eb6e5f6b927110ba", "commit_rev":
"6806c795184e4012764bf389f63f95c4e266d382"}

[commit-bot: I haz the power, 2017-04-26 16:27:38]

Description was changed from

==========
Remove ShouldBlockShelfAutoHide().

Instead, don't auto-hide the shelf if any TrayBubbleView is open.

I considered making this check if any TrayBackgroundView is active, but
some like the virtual keyboard can be active without wanting to block
the shelf from auto hiding.

BUG=686341
==========

to

==========
Remove ShouldBlockShelfAutoHide().

Instead, don't auto-hide the shelf if any TrayBubbleView is open.

I considered making this check if any TrayBackgroundView is active, but
some like the virtual keyboard can be active without wanting to block
the shelf from auto hiding.

BUG=686341

Review-Url: https://codereview.chromium.org/2831233003
Cr-Commit-Position: refs/heads/master@{#467342}
Committed:
https://chromium.googlesource.com/chromium/src/+/6806c795184e4012764bf389f63f...
==========

[commit-bot: I haz the power, 2017-04-26 16:27:40]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6806c795184e4012764bf389f63f...

[Evan Stade, 2017-04-27 01:33:49]

https://codereview.chromium.org/2831233003/diff/60001/ui/views/bubble/tray_bu...
File ui/views/bubble/tray_bubble_view.cc (right):

https://codereview.chromium.org/2831233003/diff/60001/ui/views/bubble/tray_bu...
ui/views/bubble/tray_bubble_view.cc:54: int g_current_tray_bubble_view_count_ =
0;
On 2017/04/24 15:32:40, sky wrote:
> optional: name this g_current_tray_showing_count (or something to indicate it
> corresponds to showing bubbles).

Done.