Attempting to reland the Overview Button Change.

Attempting to reland the Overview Button Change.

The inital change was landed as 256420.
The change was reverted by 256535, as a Win 7 release test failed on the build server. Another change in ash was also reverted.
I was unable to reproduce the test failure on a local Win 7 machine.

"Revert of Add a new status area control for Overview Mode. This control is only available (https://codereview.chromium.org/180743016/)"

This reverts commit 65e11004319c51eb14787d51102a3db9100da480.

BUG=337590
TBR=oshima
R=flackr

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

[jonross, 2014-03-13 13:39:13]

The initial failure on the server can be found here:

http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%282%29/buil...

A subsequent test HideTooltipWhenCursorHidden failed.

https://codereview.chromium.org/196473011/diff/1/ash/system/overview/overview...
File ash/system/overview/overview_button_tray_unittest.cc (right):

https://codereview.chromium.org/196473011/diff/1/ash/system/overview/overview...
ash/system/overview/overview_button_tray_unittest.cc:90: IsSelecting());
My concern is that I do not reset the window_selector_controller, and bring it
out of Overview Mode. However I thought that this would be addressed during test
teardown when Shell is deleted.

[flackr, 2014-03-13 14:14:53]

This has also passed win_rel ash_unittests on the CQ when landing last time. I'm
not sure what the difference between win_rel trybot and the Win7 Tests buildbot
are, but lgtm to try landing again to see if it was your change or one of the
other reverted ones.

[jonross, 2014-03-13 14:51:39]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2014-03-13 14:51:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jonross@chromium.org/196473011/1

[commit-bot: I haz the power, 2014-03-13 14:51:50]

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

[commit-bot: I haz the power, 2014-03-13 14:51:51]

Failed to apply patch for ash/ash.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ash/ash.gyp
  Hunk #1 FAILED at 369.
  Hunk #2 FAILED at 925.
  2 out of 2 hunks FAILED -- saving rejects to file ash/ash.gyp.rej

Patch:       ash/ash.gyp
Index: ash/ash.gyp
diff --git a/ash/ash.gyp b/ash/ash.gyp
index
7c3fb5b109edcd61e4a7dde01848105327c65547..3d68f71d9943bd501e603168cbf520cd98c7f796
100644
--- a/ash/ash.gyp
+++ b/ash/ash.gyp
@@ -369,6 +369,8 @@
         'system/keyboard_brightness/keyboard_brightness_control_delegate.h',
         'system/locale/locale_notification_controller.cc',
         'system/locale/locale_notification_controller.h',
+        'system/overview/overview_button_tray.h',
+        'system/overview/overview_button_tray.cc',
         'system/session/logout_button_observer.h',
         'system/session/logout_button_tray.cc',
         'system/session/logout_button_tray.h',
@@ -925,6 +927,7 @@
         'system/chromeos/tray_display_unittest.cc',
         'system/date/date_view_unittest.cc',
         'system/session/logout_confirmation_controller_unittest.cc',
+        'system/overview/overview_button_tray_unittest.cc',
         'system/session/tray_session_length_limit_unittest.cc',
         'system/tray/system_tray_unittest.cc',
         'system/tray/tray_details_view_unittest.cc',

[jonross, 2014-03-13 15:20:24]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2014-03-13 15:20:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jonross@chromium.org/196473011/2

[commit-bot: I haz the power, 2014-03-13 15:31:39]

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

[commit-bot: I haz the power, 2014-03-13 15:31:40]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[jonross, 2014-03-13 16:48:59]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2014-03-13 16:50:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jonross@chromium.org/196473011/2

[oshima, 2014-03-13 16:57:08]

On 2014/03/13 14:14:53, flackr wrote:
> This has also passed win_rel ash_unittests on the CQ when landing last time.
I'm
> not sure what the difference between win_rel trybot and the Win7 Tests
buildbot
> are, but lgtm to try landing again to see if it was your change or one of the
> other reverted ones.

It can happen if tests depends on each other through global state, and
it may or may not fail depending on the sharding scheme. (Doesn't mean this is
the case though)

[jonross, 2014-03-13 17:04:42]

If so then I can look to some extra cleanup in the unittest (line is commented.)
I had thought that the destruction of Shell would suffice to cleanup the state
of my given test. I'll keep an eye on the main build test, as the win_rel bot
passed.

On 2014/03/13 16:57:08, oshima wrote:
> On 2014/03/13 14:14:53, flackr wrote:
> > This has also passed win_rel ash_unittests on the CQ when landing last time.
> I'm
> > not sure what the difference between win_rel trybot and the Win7 Tests
> buildbot
> > are, but lgtm to try landing again to see if it was your change or one of
the
> > other reverted ones.
> 
> It can happen if tests depends on each other through global state, and
> it may or may not fail depending on the sharding scheme. (Doesn't mean this is
> the case though)

[commit-bot: I haz the power, 2014-03-14 00:15:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jonross@chromium.org/196473011/2

[commit-bot: I haz the power, 2014-03-14 05:45:25]

Change committed as 257012