Fix the bug of multiple overlays.

Fix the bug of multiple overlays.

- keyboard overlay and partial_screenshot overlay should do nothing
  if there are already overlays.
- overlay_event_filter should cancel the existing delegate if
  another delegate is trying to activate.

BUG=341958
R=jamescook@chromium.org
TEST=manually

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289199

[James Cook, 2014-08-12 20:12:02]

Thanks for fixing this!

Code looks good, but could you add a small test?

[Jun Mukai, 2014-08-12 21:13:40]

added the tests.

[James Cook, 2014-08-12 21:51:16]

LGTM!

https://codereview.chromium.org/461093003/diff/40001/ash/wm/overlay_event_fil...
File ash/wm/overlay_event_filter_unittest.cc (right):

https://codereview.chromium.org/461093003/diff/40001/ash/wm/overlay_event_fil...
ash/wm/overlay_event_filter_unittest.cc:16: TEST_F(OverlayEventFilterTest,
CancelAtActivating) {
nit: A brief comment about what you are testing/why?

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
File ash/wm/partial_screenshot_view_unittest.cc (right):

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
ash/wm/partial_screenshot_view_unittest.cc:99: // overlay. See: crbug.com/341958
nit: I've started putting the "http://" part into bug links.

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
ash/wm/partial_screenshot_view_unittest.cc:105: EXPECT_EQ(&delegate,
overlay_filter->delegate_);
nit: Maybe move up a line?

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
ash/wm/partial_screenshot_view_unittest.cc:121: }
Nice test.

[Jun Mukai, 2014-08-12 22:51:03]

https://codereview.chromium.org/461093003/diff/40001/ash/wm/overlay_event_fil...
File ash/wm/overlay_event_filter_unittest.cc (right):

https://codereview.chromium.org/461093003/diff/40001/ash/wm/overlay_event_fil...
ash/wm/overlay_event_filter_unittest.cc:16: TEST_F(OverlayEventFilterTest,
CancelAtActivating) {
On 2014/08/12 21:51:14, James Cook wrote:
> nit: A brief comment about what you are testing/why?

Done.

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
File ash/wm/partial_screenshot_view_unittest.cc (right):

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
ash/wm/partial_screenshot_view_unittest.cc:99: // overlay. See: crbug.com/341958
On 2014/08/12 21:51:14, James Cook wrote:
> nit: I've started putting the "http://" part into bug links.

Done.

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
ash/wm/partial_screenshot_view_unittest.cc:105: EXPECT_EQ(&delegate,
overlay_filter->delegate_);
On 2014/08/12 21:51:15, James Cook wrote:
> nit: Maybe move up a line?

Done.

https://codereview.chromium.org/461093003/diff/40001/ash/wm/partial_screensho...
ash/wm/partial_screenshot_view_unittest.cc:121: }
On 2014/08/12 21:51:15, James Cook wrote:
> Nice test.

thanks :)

[Jun Mukai, 2014-08-12 22:51:06]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-08-12 22:54:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mukai@chromium.org/461093003/60001

[commit-bot: I haz the power, 2014-08-13 00:34:33]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/46038)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-13 01:03:07]

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

[commit-bot: I haz the power, 2014-08-13 01:03:21]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Jun Mukai, 2014-08-13 01:31:04]

The CQ bit was checked by mukai@chromium.org

[commit-bot: I haz the power, 2014-08-13 01:43:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mukai@chromium.org/461093003/80001

[commit-bot: I haz the power, 2014-08-13 04:04:18]

Change committed as 289199