Straighten up Translate browser tests.

Straighten up Translate browser tests.

There was a potential for a subtle race built into these tests:
if the timing of determining the language shifted, the observer
may end up waiting forever. This is what's happening in
https://codereview.chromium.org/1398823004, and the race
becomes visible.

Instead, let's reogranize observation of
chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED to be managed
at the test harness level and ensure that observation begins
outside of the message loop pump.

Also, let's move a very similar test from browser_browsertest
and convert it to use the same machinery.

Finally, refactor that test to remove another subtle race:
it actually opens two tabs and receives two notifications,
but that is only vaguely implied by the test logic.

R=jam,jochen,toyoshim@chromium.org
BUG=521166
Committed: https://crrev.com/edf618bbbe9dd5f3f5034676af058b62d640998f
Cr-Commit-Position: refs/heads/master@{#357176}

[dglazkov, 2015-10-23 20:47:22]

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

[commit-bot: I haz the power, 2015-10-23 20:48:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/1

[dglazkov, 2015-10-23 20:48:31]

PTAL.

[commit-bot: I haz the power, 2015-10-23 21:22:55]

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

[commit-bot: I haz the power, 2015-10-23 21:22:56]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dglazkov, 2015-10-23 21:35:52]

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

[commit-bot: I haz the power, 2015-10-23 21:36:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/20001

[commit-bot: I haz the power, 2015-10-23 22:16:39]

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

[commit-bot: I haz the power, 2015-10-23 22:16:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dglazkov, 2015-10-23 22:31:43]

Never mind, I need to work on this some more :)

[dglazkov, 2015-10-23 23:37:32]

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

[commit-bot: I haz the power, 2015-10-23 23:38:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/40001

[commit-bot: I haz the power, 2015-10-24 00:06:59]

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

[commit-bot: I haz the power, 2015-10-24 00:07:00]

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

[dglazkov, 2015-10-24 04:54:51]

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

[commit-bot: I haz the power, 2015-10-24 04:55:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/60001

[commit-bot: I haz the power, 2015-10-24 05:09:07]

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

[commit-bot: I haz the power, 2015-10-24 05:09:08]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2015-10-24 05:26:55]

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

[dglazkov, 2015-10-24 05:26:56]

Maybe compile locally first.

[commit-bot: I haz the power, 2015-10-24 05:27:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/80001

[commit-bot: I haz the power, 2015-10-24 06:01:47]

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

[commit-bot: I haz the power, 2015-10-24 06:01:47]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2015-10-26 04:16:58]

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

[commit-bot: I haz the power, 2015-10-26 04:17:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/100001

[commit-bot: I haz the power, 2015-10-26 04:50:27]

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

[commit-bot: I haz the power, 2015-10-26 04:50:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jochen (gone - plz use gerrit), 2015-10-26 15:41:36]

lgtm

https://codereview.chromium.org/1420093004/diff/100001/chrome/browser/transla...
File chrome/browser/translate/translate_manager_browsertest.cc (right):

https://codereview.chromium.org/1420093004/diff/100001/chrome/browser/transla...
chrome/browser/translate/translate_manager_browsertest.cc:23:
TranslateManagerBrowserTest() {}
please add a virtual dtor

https://codereview.chromium.org/1420093004/diff/100001/chrome/browser/transla...
chrome/browser/translate/translate_manager_browsertest.cc:61: };
disallow copy & assign

[jam, 2015-10-26 17:20:33]

Description was changed from

==========
Straighten up Translate browser tests.

There was a potential for a subtle race built into these tests:
if the timing of determining the language shifted, the observer
may end up waiting forever. This is what's happening in
https://codereview.chromium.org/1398823004, and the race
becomes visible.

Instead, let's reogranize observation of
chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED to be managed
at the test harness level and ensure that observation begins
outside of the message loop pump.

Also, let's move a very similar test from browser_browsertest
and convert it to use the same machinery.

Finally, refactor that test to remove another subtle race:
it actually opens two tabs and receives two notifications,
but that is only vaguely implied by the test logic.

R=jam,jochen,toyoshim@chromium.org
BUG=521166
==========

to

==========
Straighten up Translate browser tests.

There was a potential for a subtle race built into these tests:
if the timing of determining the language shifted, the observer
may end up waiting forever. This is what's happening in
https://codereview.chromium.org/1398823004, and the race
becomes visible.

Instead, let's reogranize observation of
chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED to be managed
at the test harness level and ensure that observation begins
outside of the message loop pump.

Also, let's move a very similar test from browser_browsertest
and convert it to use the same machinery.

Finally, refactor that test to remove another subtle race:
it actually opens two tabs and receives two notifications,
but that is only vaguely implied by the test logic.

R=jam,jochen,toyoshim@chromium.org
BUG=521166
==========

[jam, 2015-10-26 17:20:34]

jam@chromium.org changed reviewers:
- jam@chromium.org

[jam, 2015-10-26 17:20:50]

removed myself since jochen already reviewed

[dglazkov, 2015-10-30 17:02:05]

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

[commit-bot: I haz the power, 2015-10-30 17:03:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/120001

[commit-bot: I haz the power, 2015-10-30 18:26:27]

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

[commit-bot: I haz the power, 2015-10-30 18:26:27]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2015-10-30 18:43:43]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2015-10-30 18:43:43]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1420093004/#ps140001
(title: "Feedback addressed.")

[dglazkov, 2015-10-30 18:43:53]

https://codereview.chromium.org/1420093004/diff/100001/chrome/browser/transla...
File chrome/browser/translate/translate_manager_browsertest.cc (right):

https://codereview.chromium.org/1420093004/diff/100001/chrome/browser/transla...
chrome/browser/translate/translate_manager_browsertest.cc:23:
TranslateManagerBrowserTest() {}
On 2015/10/26 at 15:41:36, jochen wrote:
> please add a virtual dtor

Done.

https://codereview.chromium.org/1420093004/diff/100001/chrome/browser/transla...
chrome/browser/translate/translate_manager_browsertest.cc:61: };
On 2015/10/26 at 15:41:36, jochen wrote:
> disallow copy & assign

This is not necessary, since the base class testing::Test already has them --
right?

[commit-bot: I haz the power, 2015-10-30 18:45:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420093004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420093004/140001

[commit-bot: I haz the power, 2015-10-30 19:47:11]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-10-30 19:48:12]

Patchset 8 (id:??) landed as
https://crrev.com/edf618bbbe9dd5f3f5034676af058b62d640998f
Cr-Commit-Position: refs/heads/master@{#357176}