Commit 1a21f92a authored by Jérôme Lebel's avatar Jérôme Lebel Committed by Chromium LUCI CQ

[iOS][EG] removeAnyOpenMenusAndInfoBars doesn't wait for views to disappear

Adding completion UUID, to make sure EG tear down waits for completion
from -[SceneController dismissModalDialogsWithCompletion:dismissOmnibox:]

Some flakiness in the tests shows that |drainUntilIdleWithTimeout:| is
not enough to make sure all views are fully dismissed before
|removeAnyOpenMenusAndInfoBars| returns.

Explanation of the flakiness:
The test failure is related to: DCHECK(!self.tabSwitcherIsActive) [1],
during the teardown.
This DCHECK is in |completionWithBVC| completion block. This block is
only used by |dismissModalDialogsWithCompletion:dismissOmnibox:| when
the tab switcher is not opened [2].

So the only way to reproduce this failure, is to open the tab switcher
while the views are being dismissed.

During the tear down, |removeAnyOpenMenusAndInfoBars| calls
|dismissModalDialogsWithCompletion:dismissOmnibox:|, and then
|closeAllTabs| is called, which opens the tab switcher.
This is why I suspect |[ChromeEarlGreyUI waitForToolbarVisible:YES]| is
needed in +[ChromeTestCase removeAnyOpenMenusAndInfoBars].

[1] https://source.chromium.org/chromium/chromium/src/+/master:ios/chrome/browser/ui/main/scene_controller.mm;l=1940
[2] https://source.chromium.org/chromium/chromium/src/+/master:ios/chrome/browser/ui/main/scene_controller.mm;l=1960

Fixed: 1143204, 1146459
Change-Id: I152850dbe5844c96ed64919a3a791f44b94f6be7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612844
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840965}
parent b8aae0c6
...@@ -53,8 +53,9 @@ id<ApplicationCommands, BrowserCommands> HandlerForActiveBrowser(); ...@@ -53,8 +53,9 @@ id<ApplicationCommands, BrowserCommands> HandlerForActiveBrowser();
// Removes all presented infobars. // Removes all presented infobars.
void RemoveAllInfoBars(); void RemoveAllInfoBars();
// Dismisses all presented views and modal dialogs. // Dismisses all presented views and modal dialogs. |completion| is invoked when
void ClearPresentedState(); // all the views are dismissed.
void ClearPresentedState(ProceduralBlock completion);
// Sets the value of a boolean local state pref. // Sets the value of a boolean local state pref.
// TODO(crbug.com/647022): Clean up other tests that use this helper function. // TODO(crbug.com/647022): Clean up other tests that use this helper function.
......
...@@ -166,8 +166,9 @@ void RemoveAllInfoBars() { ...@@ -166,8 +166,9 @@ void RemoveAllInfoBars() {
} }
} }
void ClearPresentedState() { void ClearPresentedState(ProceduralBlock completion) {
[GetForegroundActiveSceneController() dismissModalDialogsWithCompletion:nil [GetForegroundActiveSceneController()
dismissModalDialogsWithCompletion:completion
dismissOmnibox:YES]; dismissOmnibox:YES];
} }
......
...@@ -11,8 +11,10 @@ ...@@ -11,8 +11,10 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/ios/ios_util.h" #include "base/ios/ios_util.h"
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#import "base/test/ios/wait_util.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.h" #import "ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.h"
#import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
#import "ios/chrome/test/earl_grey/chrome_test_case_app_interface.h" #import "ios/chrome/test/earl_grey/chrome_test_case_app_interface.h"
#import "ios/testing/earl_grey/app_launch_manager.h" #import "ios/testing/earl_grey/app_launch_manager.h"
#import "ios/testing/earl_grey/earl_grey_test.h" #import "ios/testing/earl_grey/earl_grey_test.h"
...@@ -108,9 +110,6 @@ void ResetAuthentication() { ...@@ -108,9 +110,6 @@ void ResetAuthentication() {
[ChromeTestCaseAppInterface resetAuthentication]; [ChromeTestCaseAppInterface resetAuthentication];
} }
void RemoveInfoBarsAndPresentedState() {
[ChromeTestCaseAppInterface removeInfoBarsAndPresentedState];
}
} // namespace } // namespace
GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface) GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
...@@ -242,12 +241,20 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface) ...@@ -242,12 +241,20 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
} }
+ (void)removeAnyOpenMenusAndInfoBars { + (void)removeAnyOpenMenusAndInfoBars {
RemoveInfoBarsAndPresentedState(); NSUUID* uuid = [NSUUID UUID];
// After programatically removing UI elements, allow Earl Grey's // Removes all the UI elements.
// UI synchronization to become idle, so subsequent steps won't start before [ChromeTestCaseAppInterface
// the UI is in a good state. removeInfoBarsAndPresentedStateWithCallbackUUID:uuid];
[[GREYUIThreadExecutor sharedInstance] ConditionBlock condition = ^{
drainUntilIdleWithTimeout:kDrainTimeout]; return [ChromeTestCaseAppInterface isCallbackInvokedWithUUID:uuid];
};
NSString* errorMessage =
@"+[ChromeTestCaseAppInterface "
@"removeInfoBarsAndPresentedStateWithCallbackUUID:] callback failed";
// Waits until the UI elements are removed.
bool callbackInvoked = base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, condition);
GREYAssertTrue(callbackInvoked, errorMessage);
} }
+ (void)closeAllTabs { + (void)closeAllTabs {
......
...@@ -19,7 +19,14 @@ ...@@ -19,7 +19,14 @@
+ (void)resetAuthentication; + (void)resetAuthentication;
// Removes all infobars and clears any presented state. // Removes all infobars and clears any presented state.
+ (void)removeInfoBarsAndPresentedState; // See +[ChromeTestCaseAppInterface isCallbackInvokedWithUUID:] to know when
// all views are dismissed.
+ (void)removeInfoBarsAndPresentedStateWithCallbackUUID:(NSUUID*)callbackUUID;
// Returns YES if the callback related to |callbackUUID| has been invoked. Once
// this method returns YES, |callbackUUID| is dropped, and a second call will
// return NO.
+ (BOOL)isCallbackInvokedWithUUID:(NSUUID*)callbackUUID;
@end @end
......
...@@ -4,13 +4,22 @@ ...@@ -4,13 +4,22 @@
#import "ios/chrome/test/earl_grey/chrome_test_case_app_interface.h" #import "ios/chrome/test/earl_grey/chrome_test_case_app_interface.h"
#import "base/check.h"
#import "ios/chrome/test/app/chrome_test_util.h" #import "ios/chrome/test/app/chrome_test_util.h"
#include "ios/chrome/test/app/signin_test_util.h" #import "ios/chrome/test/app/signin_test_util.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
namespace {
// Stores the callback UUIDs when the callback is invoked. The UUIDs can be
// checked with +[ChromeTestCaseAppInterface isCallbackInvokedWithUUID:].
NSMutableSet* invokedCallbackUUID = nil;
}
@implementation ChromeTestCaseAppInterface @implementation ChromeTestCaseAppInterface
+ (void)setUpMockAuthentication { + (void)setUpMockAuthentication {
...@@ -26,9 +35,28 @@ ...@@ -26,9 +35,28 @@
chrome_test_util::ResetMockAuthentication(); chrome_test_util::ResetMockAuthentication();
} }
+ (void)removeInfoBarsAndPresentedState { + (void)removeInfoBarsAndPresentedStateWithCallbackUUID:(NSUUID*)callbackUUID {
chrome_test_util::RemoveAllInfoBars(); chrome_test_util::RemoveAllInfoBars();
chrome_test_util::ClearPresentedState(); chrome_test_util::ClearPresentedState(^() {
if (callbackUUID)
[self callbackInvokedWithUUID:callbackUUID];
});
}
+ (BOOL)isCallbackInvokedWithUUID:(NSUUID*)callbackUUID {
if (![invokedCallbackUUID containsObject:callbackUUID])
return NO;
[invokedCallbackUUID removeObject:callbackUUID];
return YES;
}
#pragma mark - Private
+ (void)callbackInvokedWithUUID:(NSUUID*)callbackUUID {
if (!invokedCallbackUUID)
invokedCallbackUUID = [NSMutableSet set];
DCHECK(![invokedCallbackUUID containsObject:callbackUUID]);
[invokedCallbackUUID addObject:callbackUUID];
} }
@end @end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment