Check that tab switcher is active/inactive before visibility check.

ios/chrome/browser/ui/stack_view/stack_view_egtest.mm

Index: ios/chrome/browser/ui/stack_view/stack_view_egtest.mm
diff --git a/ios/chrome/browser/ui/stack_view/stack_view_egtest.mm b/ios/chrome/browser/ui/stack_view/stack_view_egtest.mm
index 6d2f88fcccce4ea72a79d2f603bd590b1b6bd6a0..ad9486b7450ea51e30c7522dfdd6c90a96d2c2eb 100644
--- a/ios/chrome/browser/ui/stack_view/stack_view_egtest.mm
+++ b/ios/chrome/browser/ui/stack_view/stack_view_egtest.mm
@@ -24,6 +24,7 @@
 #import "ios/chrome/test/earl_grey/chrome_matchers.h"
 #import "ios/chrome/test/earl_grey/chrome_test_case.h"
 #include "ios/testing/earl_grey/disabled_test_macros.h"
+#import "ios/testing/wait_util.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -50,8 +51,28 @@
   return ViewMatchingView([chrome_test_util::GetStackViewController() view]);
 }
 
+// Waits for the Stack View to be active/inactive.
+void WaitForStackViewActive(bool active) {

[Eugene But (OOO till 7-30), 2017/03/07 16:08:15]

Do you want to return bool from this method and assert that result is true?

[kkhorimoto, 2017/03/07 19:05:03]

The condition we're checking below is dependent on |active|, so we'd need to
have two separate functions for active or inactive.  Moreover, we'd need to
implement the GREYConditions at the callsites instead of having it encapsulated
by this function.

[Eugene But (OOO till 7-30), 2017/03/07 19:21:12]

I did a search for WaitFor methods in EG tests and most of them assert on
timeout, which sounds like a reasonable thing to do in this case as well.
WaitForStackViewActive should not be not-op if condition is not met. Doing so
causes bugs like this:
https://bugs.chromium.org/p/chromium/issues/detail?id=685439

[kkhorimoto, 2017/03/07 21:19:50]

Fixed

+  // Wait for the tab switcher to be active or inactive.
+  NSString* activeTabSwitcherDescription =
+      [NSString stringWithFormat:@"Waiting for tab switcher to be %@.",
+                                 active ? @"active" : @"inactive"];
+  BOOL (^activeTabSwitcherBlock)
+  () = ^BOOL {
+    BOOL isActive = chrome_test_util::GetStackViewController() &&
+                    chrome_test_util::IsTabSwitcherActive();
+    return active ? isActive : !isActive;
+  };
+  GREYCondition* activeTabSwitcherCondition =
+      [GREYCondition conditionWithName:activeTabSwitcherDescription
+                                 block:activeTabSwitcherBlock];
+  [activeTabSwitcherCondition
+      waitWithTimeout:testing::kWaitForUIElementTimeout];
+}
+
 // Waits for the Stack View to be visible/hidden.
 void CheckForStackViewVisibility(bool visible) {
+  // Verify the visibility of the stack view.

[Eugene But (OOO till 7-30), 2017/03/07 16:08:15]

Could you please promote this comment to a function comment. Current function
comment is not correct as it does not wait.

[kkhorimoto, 2017/03/07 19:05:03]

Done.

   id<GREYMatcher> visibilityMatcher =
       grey_allOf(visible ? grey_sufficientlyVisible() : grey_notVisible(),
                  visible ? grey_notNil() : grey_nil(), nil);
@@ -70,6 +91,7 @@ void OpenStackView() {
   [[EarlGrey selectElementWithMatcher:stackButtonMatcher]
       performAction:grey_tap()];
   // Verify that a StackViewController was presented.
+  WaitForStackViewActive(true);
   CheckForStackViewVisibility(true);
 }
 
@@ -105,6 +127,7 @@ void OpenNewTabUsingStackView() {
   ShowDeckWithType(DeckType::NORMAL);
   [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"New Tab")]
       performAction:grey_tap()];
+  WaitForStackViewActive(false);
   CheckForStackViewVisibility(false);
 }
 
@@ -121,6 +144,7 @@ void OpenNewIncognitoTabUsingStackView() {
   NSString* newIncognitoTabID = kToolsMenuNewIncognitoTabId;
   [[EarlGrey selectElementWithMatcher:grey_accessibilityID(newIncognitoTabID)]
       performAction:grey_tap()];
+  WaitForStackViewActive(false);
   CheckForStackViewVisibility(false);
 }
 
@@ -137,6 +161,7 @@ void SelectTabUsingStackView(Tab* tab) {
   [[EarlGrey selectElementWithMatcher:ViewMatchingView(card_title_label)]
       performAction:grey_tap()];
   // Wait for the StackViewController to be dismissed.
+  WaitForStackViewActive(false);
   CheckForStackViewVisibility(false);
   // Checks that the next Tab has been selected.
   GREYAssertEqual(tab, chrome_test_util::GetCurrentTab(),
@@ -187,13 +212,7 @@ - (void)testCloseTab {
 }
 
 // Tests closing all Tabs in the stack view.
-// TODO(crbug.com/693517): Re-enable this test on simulator.
-#if TARGET_IPHONE_SIMULATOR
-#define MAYBE_testCloseAllTabs FLAKY_testCloseAllTabs
-#else
-#define MAYBE_testCloseAllTabs testCloseAllTabs
-#endif
-- (void)MAYBE_testCloseAllTabs {
+- (void)testCloseAllTabs {
   // The StackViewController is only used on iPhones.
   if (IsIPadIdiom())
     EARL_GREY_TEST_SKIPPED(@"Stack view is not used on iPads.");