Commit 480a1ae6 authored by Rohit Rao's avatar Rohit Rao Committed by Commit Bot

[ios] Works around an iOS10 bug where UIVCs are over-dismissed.

The combination of UIDocumentMenuViewController and WKFileUploadPanel ends up
calling |dismissViewController:animated:| twice.  The second call triggers a
DCHECK that's intended to protect the BrowserViewController from being
accidentally dismissed.

This CL works around the bug by detecting when a UIDocumentMenuViewController is
presented and ignoring spurious calls to dismiss.

BUG=801165

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ifa957dd8f0c387537130754335ec6bf137d68b2b
Reviewed-on: https://chromium-review.googlesource.com/895727Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534432}
parent 45b2c85b
...@@ -621,6 +621,11 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -621,6 +621,11 @@ NSString* const kBrowserViewControllerSnackbarCategory =
// is used to determine whether the pre-rendering animation should be played // is used to determine whether the pre-rendering animation should be played
// or not. // or not.
BOOL _insertedTabWasPrerenderedTab; BOOL _insertedTabWasPrerenderedTab;
// A pointer to the most recently presented UIDocumentMenuViewController.
// Used to work around https://crbug.com/801165.
__weak UIDocumentMenuViewController*
_lastPresentedUIDocumentMenuViewController;
} }
// The browser's side swipe controller. Lazily instantiated on the first call. // The browser's side swipe controller. Lazily instantiated on the first call.
...@@ -1776,6 +1781,16 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint { ...@@ -1776,6 +1781,16 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
- (void)dismissViewControllerAnimated:(BOOL)flag - (void)dismissViewControllerAnimated:(BOOL)flag
completion:(void (^)())completion { completion:(void (^)())completion {
if (!base::ios::IsRunningOnIOS11OrLater() &&
_lastPresentedUIDocumentMenuViewController &&
!self.presentedViewController) {
// TODO(crbug.com/801165): On iOS10, UIDocumentMenuViewController and
// WKFileUploadPanel somehow combine to call dismiss twice instead of once.
// The second call would dismiss the BVC itself, so look for that case and
// return early.
return;
}
// It is an error to call this method when no VC is being presented. // It is an error to call this method when no VC is being presented.
DCHECK(!TabSwitcherPresentsBVCEnabled() || self.presentedViewController); DCHECK(!TabSwitcherPresentsBVCEnabled() || self.presentedViewController);
...@@ -1862,6 +1877,20 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint { ...@@ -1862,6 +1877,20 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
[self.sideSwipeController resetContentView]; [self.sideSwipeController resetContentView];
} }
// TODO(crbug.com/801165): Workaround for an iOS10 bug.
// UIDocumentMenuViewController, when used via WKFileUploadPanel,
// over-dismisses itself, which triggers a DCHECK in
// dismissViewController:animated:. If a UIDocumentMenuViewController is
// being presented here, save a weak pointer that can be tested later when
// dismiss is called.
if (!base::ios::IsRunningOnIOS11OrLater() &&
[viewControllerToPresent
isKindOfClass:[UIDocumentMenuViewController class]]) {
_lastPresentedUIDocumentMenuViewController =
base::mac::ObjCCastStrict<UIDocumentMenuViewController>(
viewControllerToPresent);
}
[super presentViewController:viewControllerToPresent [super presentViewController:viewControllerToPresent
animated:flag animated:flag
completion:finalCompletionHandler]; completion:finalCompletionHandler];
......
...@@ -80,4 +80,28 @@ ...@@ -80,4 +80,28 @@
[ChromeEarlGrey waitForWebViewContainingText:responses[destinationURL]]; [ChromeEarlGrey waitForWebViewContainingText:responses[destinationURL]];
} }
// Tests the fix for the regression reported in https://crbug.com/801165. The
// bug was triggered by opening an HTML file picker and then dismissing it.
- (void)testFixForCrbug801165 {
if (IsIPadIdiom()) {
EARL_GREY_TEST_SKIPPED(@"Skipped for iPad (no action sheet on tablet)");
}
std::map<GURL, std::string> responses;
const GURL testURL = web::test::HttpServer::MakeUrl("http://origin");
responses[testURL] = "File Picker Test <input id=\"file\" type=\"file\">";
web::test::SetUpSimpleHttpServer(responses);
// Load the test page.
[ChromeEarlGrey loadURL:testURL];
[ChromeEarlGrey waitForWebViewContainingText:"File Picker Test"];
// Invoke the file picker and tap on the "Cancel" button to dismiss the file
// picker.
[ChromeEarlGrey tapWebViewElementWithID:@"file"];
[[EarlGrey selectElementWithMatcher:chrome_test_util::CancelButton()]
performAction:grey_tap()];
[[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
}
@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