App crashes when switching between tabs with download infobar present. |
|||||||||||
Issue descriptionApp Version: 69.0.3450.0 canary iOS Version: 11.2.6, 12 beta1 Device: iPhoneX, iPhone8 URL: developer.apple.com/fonts Steps to reproduce: 1. Launch Google Chrome 2. Navigate to techmeme.com in tab#1 3. Navigate to developer.apple.com/fonts in tab#2 4. Swipe on bottom toolbar to switch to tab#1 5. Swipe on bottom toolbar again to switch to tab#2 Observed results: App Crashes. Crash ID: http://crash/a7df32079b0893a0 Working fine if UI-Refresh-Phase1 flag is disabled. Expected results: App shouldn't crash. Number of times you were able to reproduce: 5/5 Bug reproducible after clean install: Yes Bug reproducible after clearing cache and cookies: Yes 0x000000018277b164 (CoreFoundation + 0x00143164 ) __exceptionPreprocess 0x00000001819c4524 (libobjc.A.dylib + 0x00008524 ) objc_exception_throw 0x000000018277b0a8 (CoreFoundation + 0x001430a8 ) +[NSException raise:format:] 0x00000001832958f0 (Foundation + 0x0022c8f0 ) -[NSLayoutConstraint _setActive:mutuallyExclusiveConstraints:] 0x0000000183295bdc (Foundation + 0x0022cbdc ) __55+[NSLayoutConstraint _addOrRemoveConstraints:activate:]_block_invoke 0x00000001830b74d8 (Foundation + 0x0004e4d8 ) -[NSISEngine withBehaviors:performModifications:] 0x0000000183295aac (Foundation + 0x0022caac ) +[NSLayoutConstraint _addOrRemoveConstraints:activate:] 0x0000000102a99104 (Chrome -download_manager_view_controller.mm:214 ) -[DownloadManagerViewController updateViewConstraints] 0x000000018c83d9b4 (UIKit + 0x00b179b4 ) -[UIView(AdditionalLayoutSupport) _sendUpdateConstraintsIfNecessaryForSecondPass:] 0x000000018c83de7c (UIKit + 0x00b17e7c ) -[UIView(AdditionalLayoutSupport) _updateConstraintsIfNeededCollectingViews:forSecondPass:] 0x000000018c83dd54 (UIKit + 0x00b17d54 ) -[UIView(AdditionalLayoutSupport) _updateConstraintsIfNeededCollectingViews:forSecondPass:] 0x00000001830b74d8 (Foundation + 0x0004e4d8 ) -[NSISEngine withBehaviors:performModifications:] 0x000000018c83e614 (UIKit + 0x00b18614 ) __100-[UIView(AdditionalLayoutSupport) _updateConstraintsIfNeededWithViewForVariableChangeNotifications:]_block_invoke 0x000000018c83d200 (UIKit + 0x00b17200 ) -[UIView(AdditionalLayoutSupport) _withUnsatisfiableConstraintsLoggingSuspendedIfEngineDelegateExists:] 0x000000018c83e230 (UIKit + 0x00b18230 ) -[UIView(AdditionalLayoutSupport) _updateConstraintsIfNeededWithViewForVariableChangeNotifications:] 0x000000018c83f25c (UIKit + 0x00b1925c ) -[UIView(AdditionalLayoutSupport) _updateConstraintsAtEngineLevelIfNeededWithViewForVariableChangeNotifications:] 0x000000018bd47d38 (UIKit + 0x00021d38 ) -[UIView(Hierarchy) layoutBelowIfNeeded] 0x000000010273ce3c (Chrome -vertical_animation_container.mm:68 ) -[VerticalAnimationContainer prepareForPresentation] 0x0000000102a940dc (Chrome -download_manager_coordinator.mm:167 ) -[DownloadManagerCoordinator start] 0x0000000102a945d8 (Chrome -download_manager_coordinator.mm:260 ) -[DownloadManagerCoordinator downloadManagerTabHelper:didShowDownload:] 0x0000000102531284 (Chrome -download_manager_tab_helper.mm:73 ) DownloadManagerTabHelper::WasShown(web::WebState*) 0x00000001024b4f90 (Chrome -web_state_impl.mm:602 ) web::WebStateImpl::WasShown() 0x0000000102a61004 (Chrome -browser_view_controller.mm:2469 ) -[BrowserViewController displayTab:] 0x0000000102a63c28 (Chrome -browser_view_controller.mm:2994 ) -[BrowserViewController tabSelected:notifyToolbar:] 0x0000000102a6dbc8 (Chrome -browser_view_controller.mm:4948 ) -[BrowserViewController tabModel:didChangeActiveTab:previousTab:atIndex:] 0x0000000182782acc (CoreFoundation + 0x0014aacc ) __invoking___ 0x0000000182661368 (CoreFoundation + 0x00029368 ) -[NSInvocation invoke] 0x0000000182665e18 (CoreFoundation + 0x0002de18 ) -[NSInvocation invokeWithTarget:] 0x0000000102cc80f4 (Chrome -crb_protocol_observers.mm:169 ) -[CRBProtocolObservers forwardInvocation:] 0x0000000182780818 (CoreFoundation + 0x00148818 ) ___forwarding___ 0x0000000182665cc8 (CoreFoundation + 0x0002dcc8 ) _CF_forwarding_prep_0 0x0000000102c39a08 (Chrome -tab_model_observers_bridge.mm:97 ) -[TabModelObserversBridge webStateList:didChangeActiveWebState:oldWebState:atIndex:reason:] 0x000000010260d61c (Chrome -web_state_list_observer_bridge.mm:117 ) WebStateListObserverBridge::WebStateActivatedAt(WebStateList*, web::WebState*, web::WebState*, int, int) 0x000000010260bdf8 (Chrome -web_state_list.mm:342 ) WebStateList::NotifyIfActiveWebStateChanged(web::WebState*, int) 0x0000000102c33c0c (Chrome -tab_model.mm:292 ) -[TabModel setCurrentTab:] 0x0000000102746b8c (Chrome -card_side_swipe_view.mm:476 ) __30-[CardSideSwipeView finishPan]_block_invoke.182 0x000000018bd6e184 (UIKit + 0x00048184 ) -[UIViewAnimationBlockDelegate _didEndBlockAnimation:finished:context:] 0x000000018bd6daf8 (UIKit + 0x00047af8 ) -[UIViewAnimationState sendDelegateAnimationDidStop:finished:] 0x000000018bd6d804 (UIKit + 0x00047804 ) -[UIViewAnimationState animationDidStop:finished:] 0x00000001867cedc0 (QuartzCore + 0x00131dc0 ) CA::Layer::run_animation_callbacks(void*) 0x00000001820faa10 (libdispatch.dylib + 0x00001a10 ) _dispatch_client_callout 0x000000018213bc7c (libdispatch.dylib + 0x00042c7c ) _dispatch_main_queue_callback_4CF$VARIANT$armv81 0x0000000182723340 (CoreFoundation + 0x000eb340 ) __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ 0x0000000182720f1c (CoreFoundation + 0x000e8f1c ) __CFRunLoopRun 0x0000000182640c54 (CoreFoundation + 0x00008c54 ) CFRunLoopRunSpecific 0x00000001844ecf80 (GraphicsServices + 0x0000af80 ) GSEventRunModal 0x000000018bd995c0 (UIKit + 0x000735c0 ) UIApplicationMain 0x00000001022fd010 (Chrome -chrome_exe_main.mm:54 ) main 0x0000000182160568 (libdyld.dylib + 0x00001568 ) start
,
Jun 5 2018
,
Jun 5 2018
,
Jun 5 2018
*** Terminating app due to uncaught exception 'NSGenericException', reason: 'Unable to activate constraint with anchors <NSLayoutDimension:0x624000275400 "UILayoutGuide:0x6100001b8100''.height"> and <NSLayoutDimension:0x610000279640 "NamedGuide:0x7fad1c438f10''.height"> because they have no common ancestor. DownloadManager and SecondaryToolbar don't have a common view. DownloadManager is a subview of browserContainerCoordinator's view. And browserContainerCoordinator is not in a view hierarchy. Gauthier, do you understand what's wrong with BVC's view hierarchy?
,
Jun 7 2018
,
Jun 8 2018
,
Jun 18 2018
At this point the BrowserContainerViewController is directly presented by the TabGridViewController (if you do "po [self.baseViewController presentingViewController]" in the VerticalAnimationContainer, you can see it. Put a breakpoint in DownloadManagerViewController -updateViewConstraints for easier debugging). I have no idea why this is happening, especially considering this is triggered by BrowserViewController -displayTab:. I think it is related to Rohit and Mark work to have the TabGrid as the underlying VC for the app. Adding a bunch of people to see if they understand why.
,
Jul 16
We have the following code in SideSwipeController:
// Remove content area so it doesn't receive any pan events.
[[swipeDelegate_ sideSwipeContentView] removeFromSuperview];
And in [CardSideSwipeView finishPan]:
completion:^(BOOL finished) {
// Changing the model even when the tab is the same at the end of the
// animation allows the UI to recover.
[model_ setCurrentTab:destinationTab];
...
[_delegate sideSwipeViewDismissAnimationDidEnd:self];
}];
So we're removing the BrowserContainerViewController's view when we start a swipe, and we add it backat the end, in the call to sideSwipeViewDismissAnimationDidEnd, but the DownloadManager code is invoked as a result of the call to setCurrentTab. So at the point where we try to set up the download bar, the BrowserContainerViewController's view isn't yet in the view hierarchy.
,
Jul 16
Two options, both of which seem to work under a trivial test, but might break something horribly: 1) Stop calling removeFromSuperview, and accept whatever behavior that code was designed to prevent. Potentially call setUserInteractionEnabled:NO instead. 2) Reorder the calls to setCurrentTab and sideSwipeViewDismissAnimationDidEnd, so that the view hierarchy is properly reassembled before tabs are swapped in.
,
Jul 16
- (void)sideSwipeViewDismissAnimationDidEnd:(UIView*)sideSwipeView {
DCHECK(![self canShowTabStrip]);
// Update frame incase orientation changed while |contentArea| was out of
// the view hierarchy.
self.contentArea.frame = sideSwipeView.frame;
[self.view insertSubview:self.contentArea aboveSubview:_fakeStatusBarView];
[self updateVoiceSearchBarVisibilityAnimated:NO];
[self updateToolbar];
// Reset horizontal stack view.
[sideSwipeView removeFromSuperview];
[self.sideSwipeController setInSwipe:NO];
[_infoBarContainer->view() setHidden:NO];
}
The interesting calls above are:
1) updateVoiceSearchBarVisibilityAnimated
2) updateToolbar
#1 is called directly by didChangeActiveTab.
#2 is called via didChangeActiveTab->tabSelected:notifyToolbar:->displayTab.
So both of the calls that update visible UI would be called again after changing the active tab, even if we swapped the order of calls. I don't see any immediately obvious reasons why this would fail.
,
Jul 17
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b215a0895e0253ba829eb665459030d8b2f4234b commit b215a0895e0253ba829eb665459030d8b2f4234b Author: Rohit Rao <rohitrao@chromium.org> Date: Tue Jul 17 16:21:06 2018 [ios] Fixes a crash when displaying the downloads UI after a side swipe. During a side swipe to change tabs, we remove the content area from the view hierarchy. This was causing issues with the downloads UI, as we were attempting to set up constraints before the view hierarchy was reassembled. This CL swaps the order of calls to sideSwipeViewDismissAnimationDidEnd and setCurrentTab, to ensure that the view hierarchy is fully reassembled before any of the tab change machinery is invoked. BUG= 849827 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Iba2642f949a10d1a31ed32ae3763fed244022fdf Reviewed-on: https://chromium-review.googlesource.com/1138351 Reviewed-by: Mark Cogan <marq@chromium.org> Reviewed-by: Justin Cohen <justincohen@chromium.org> Commit-Queue: Rohit Rao <rohitrao@chromium.org> Cr-Commit-Position: refs/heads/master@{#575664} [modify] https://crrev.com/b215a0895e0253ba829eb665459030d8b2f4234b/ios/chrome/browser/ui/side_swipe/card_side_swipe_view.mm
,
Jul 18
,
Jul 24
Verified on M70.0.3501.0 canary Device: iPhoneSE, iPhone7Plus, iPhoneX, iOS: 11.4.1, 10.3.3, 12.0 beta#4 No crashes seen and able to switch between tabs. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by srikanthg@chromium.org
, Jun 5 2018