Re-enable tabs_interactive_uitest for linux_aura

Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) change BringBrowserWindowToFront with BrowserActivationWaiter support
(2) enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests

Committed: https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45
Cr-Commit-Position: refs/heads/master@{#432584}

[Qiang(Joe) Xu, 2016-11-15 00:48:23]

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

[commit-bot: I haz the power, 2016-11-15 00:49:22]

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

[Qiang(Joe) Xu, 2016-11-15 00:52:56]

Description was changed from

==========
Re-enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests
==========

to

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) create a helper method ActivateBrowserWindow in interactive_test_utils
(2) enable tabs_interactive_uitest for linux_aura
(3) utilize ActivateBrowserWindow in browser_focus_uitest

BUG=665233
TEST=automate tests
==========

[Qiang(Joe) Xu, 2016-11-15 00:53:05]

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

[Qiang(Joe) Xu, 2016-11-15 02:17:22]

Hi Scott, coule you ptal? Thanks!

https://codereview.chromium.org/2500093003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/tabs/tabs_interactive_test.cc (right):

https://codereview.chromium.org/2500093003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/tabs/tabs_interactive_test.cc:42:
AddTabAtIndexToBrowser(new_browser, 0, url, ui::PAGE_TRANSITION_LINK, true);
Add these two lines, otherwise test fails on linux.

[commit-bot: I haz the power, 2016-11-15 02:47:43]

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

[commit-bot: I haz the power, 2016-11-15 02:47:44]

Dry run: This issue passed the CQ dry run.

[sky, 2016-11-15 03:47:01]

https://codereview.chromium.org/2500093003/diff/1/chrome/test/base/interactiv...
File chrome/test/base/interactive_test_utils.cc (right):

https://codereview.chromium.org/2500093003/diff/1/chrome/test/base/interactiv...
chrome/test/base/interactive_test_utils.cc:27:
ASSERT_TRUE(BringBrowserWindowToFront(browser));
You should make ActivateBrowserWindow return a status and have callers
ASSERT_TRUE on it. Less mysterious failures that way.

[Qiang(Joe) Xu, 2016-11-15 06:39:41]

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

[commit-bot: I haz the power, 2016-11-15 06:39:56]

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, 2016-11-15 08:02:37]

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

[commit-bot: I haz the power, 2016-11-15 08:02:38]

Dry run: This issue passed the CQ dry run.

[Qiang(Joe) Xu, 2016-11-15 16:25:30]

Hi Scott, new patch ready, ptal, thanks!

https://codereview.chromium.org/2500093003/diff/1/chrome/test/base/interactiv...
File chrome/test/base/interactive_test_utils.cc (right):

https://codereview.chromium.org/2500093003/diff/1/chrome/test/base/interactiv...
chrome/test/base/interactive_test_utils.cc:27:
ASSERT_TRUE(BringBrowserWindowToFront(browser));
On 2016/11/15 03:47:00, sky wrote:
> You should make ActivateBrowserWindow return a status and have callers
> ASSERT_TRUE on it. Less mysterious failures that way.

Yes, I agree. Done.

[sky, 2016-11-15 17:45:12]

https://codereview.chromium.org/2500093003/diff/40001/chrome/test/base/intera...
File chrome/test/base/interactive_test_utils.h (right):

https://codereview.chromium.org/2500093003/diff/40001/chrome/test/base/intera...
chrome/test/base/interactive_test_utils.h:30: bool
BringBrowserWindowToFront(const Browser* browser) WARN_UNUSED_RESULT;
Aren't ActivateBrowserWindow and BringBrowserWindowToFront the same? They both
should change activation, right?

[Qiang(Joe) Xu, 2016-11-15 18:12:36]

Patchset #4 (id:60001) has been deleted

[Qiang(Joe) Xu, 2016-11-15 18:16:44]

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

[commit-bot: I haz the power, 2016-11-15 18:17:31]

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

[Qiang(Joe) Xu, 2016-11-15 18:17:36]

Patchset #4 (id:80001) has been deleted

[commit-bot: I haz the power, 2016-11-15 18:20:58]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[commit-bot: I haz the power, 2016-11-15 18:26:15]

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

[commit-bot: I haz the power, 2016-11-15 18:26:16]

Dry run: This issue passed the CQ dry run.

[Qiang(Joe) Xu, 2016-11-15 18:29:19]

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

[commit-bot: I haz the power, 2016-11-15 18:29:55]

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

[Qiang(Joe) Xu, 2016-11-15 19:01:26]

Hi Scott, new patch ready, ptal.

https://codereview.chromium.org/2500093003/diff/40001/chrome/test/base/intera...
File chrome/test/base/interactive_test_utils.h (right):

https://codereview.chromium.org/2500093003/diff/40001/chrome/test/base/intera...
chrome/test/base/interactive_test_utils.h:30: bool
BringBrowserWindowToFront(const Browser* browser) WARN_UNUSED_RESULT;
On 2016/11/15 17:45:12, sky wrote:
> Aren't ActivateBrowserWindow and BringBrowserWindowToFront the same? They both
> should change activation, right?

yes.. sorry for not noticing that : )

[sky, 2016-11-15 20:21:12]

https://codereview.chromium.org/2500093003/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/api/tabs/tabs_interactive_test.cc (right):

https://codereview.chromium.org/2500093003/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/tabs/tabs_interactive_test.cc:135:
ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
Is this really necessary? interactive_ui_tests_main.cc does for browser
(assuming not mac).

https://codereview.chromium.org/2500093003/diff/100001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_focus_uitest.cc (left):

https://codereview.chromium.org/2500093003/diff/100001/chrome/browser/ui/brow...
chrome/browser/ui/browser_focus_uitest.cc:334:
ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
I don't think any of these calls to BringBrowserWindowToFront() are necessary.

https://codereview.chromium.org/2500093003/diff/100001/chrome/test/base/inter...
File chrome/test/base/interactive_test_utils.h (right):

https://codereview.chromium.org/2500093003/diff/100001/chrome/test/base/inter...
chrome/test/base/interactive_test_utils.h:24: // Brings the native window for
|browser| to the foreground. If the internal
How about modifying the first sentence to:
"Brings the native window for |browser| to the foreground and waits until the
browser is active."

And remove the second sentence.

[commit-bot: I haz the power, 2016-11-15 20:26:35]

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

[commit-bot: I haz the power, 2016-11-15 20:26:36]

Dry run: This issue passed the CQ dry run.

[Qiang(Joe) Xu, 2016-11-15 22:40:23]

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

[commit-bot: I haz the power, 2016-11-15 22:40:56]

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

[Qiang(Joe) Xu, 2016-11-15 22:42:29]

Hi Scott, here is the update. But I am little puzzled on removing
BringBrowserWindowToFront(browser()). It seems conflicted with previous code
comments.

https://codereview.chromium.org/2500093003/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/api/tabs/tabs_interactive_test.cc (right):

https://codereview.chromium.org/2500093003/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/tabs/tabs_interactive_test.cc:135:
ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
On 2016/11/15 20:21:11, sky wrote:
> Is this really necessary? interactive_ui_tests_main.cc does for browser
> (assuming not mac). 

Yes, I saw it was done for all platforms except mac. I concluded that only
creating new_browser window in the test needs this call.
However, the previous code: BrowserFocusTest.FocusOnNavigate has comment "Needed
on Mac", which is what I am worried. I don't understand why it is needed there.

https://codereview.chromium.org/2500093003/diff/100001/chrome/test/base/inter...
File chrome/test/base/interactive_test_utils.h (right):

https://codereview.chromium.org/2500093003/diff/100001/chrome/test/base/inter...
chrome/test/base/interactive_test_utils.h:24: // Brings the native window for
|browser| to the foreground. If the internal
On 2016/11/15 20:21:12, sky wrote:
> How about modifying the first sentence to:
> "Brings the native window for |browser| to the foreground and waits until the
> browser is active."
> 
> And remove the second sentence.

Done.

https://codereview.chromium.org/2500093003/diff/120001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_focus_uitest.cc (right):

https://codereview.chromium.org/2500093003/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_focus_uitest.cc:677: // Needed on Mac.
I didn't remove this, as it was there before:
https://codereview.chromium.org/2488383003/. So I am worried, if removing all
the above, it may cause mac tests flaky.

[commit-bot: I haz the power, 2016-11-15 23:09:55]

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

[commit-bot: I haz the power, 2016-11-15 23:09:55]

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

[sky, 2016-11-16 01:45:15]

It would be good if someone could make the mac side work. Sadly I don't have a
mac. LgTM

[Qiang(Joe) Xu, 2016-11-16 06:29:02]

The CQ bit was checked by warx@chromium.org

[Qiang(Joe) Xu, 2016-11-16 06:29:03]

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/2500093003/#ps140001
(title: "add TODO")

[commit-bot: I haz the power, 2016-11-16 06:29:34]

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-11-16 08:53:30]

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

[commit-bot: I haz the power, 2016-11-16 08:53:31]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Qiang(Joe) Xu, 2016-11-16 17:07:25]

The CQ bit was checked by warx@chromium.org

[commit-bot: I haz the power, 2016-11-16 17:08:10]

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

[Qiang(Joe) Xu, 2016-11-16 17:48:28]

Description was changed from

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) create a helper method ActivateBrowserWindow in interactive_test_utils
(2) enable tabs_interactive_uitest for linux_aura
(3) utilize ActivateBrowserWindow in browser_focus_uitest

BUG=665233
TEST=automate tests
==========

to

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) change BringBrowserWindowToFront with BrowserActivationWaiter support
(2) enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests
==========

[commit-bot: I haz the power, 2016-11-16 19:14:34]

Description was changed from

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) change BringBrowserWindowToFront with BrowserActivationWaiter support
(2) enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests
==========

to

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) change BringBrowserWindowToFront with BrowserActivationWaiter support
(2) enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests
==========

[commit-bot: I haz the power, 2016-11-16 19:14:37]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-11-16 19:53:29]

Description was changed from

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) change BringBrowserWindowToFront with BrowserActivationWaiter support
(2) enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests
==========

to

==========
Re-enable tabs_interactive_uitest for linux_aura

Changes include:
(1) change BringBrowserWindowToFront with BrowserActivationWaiter support
(2) enable tabs_interactive_uitest for linux_aura

BUG=665233
TEST=automate tests

Committed: https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45
Cr-Commit-Position: refs/heads/master@{#432584}
==========

[commit-bot: I haz the power, 2016-11-16 19:53:30]

Patchset 6 (id:??) landed as
https://crrev.com/3e256ad41fdb6711f19c85cc80a7c860304a9b45
Cr-Commit-Position: refs/heads/master@{#432584}