DCHECK when restoring tabs after crash ("Chromium didn't shut down correctly"). |
||||||||||
Issue descriptionLaunch Chrome, visit a page (for example "https://mhahmadi.github.io/pr-demo/"), kill the app (for example stop the debugger), then restart the app and press "Restore" button when presented with the infobar "Chromium didn't shut down correctly". Expected result: page load without any DCHECK. Result: DCHECK failure: [0517/164950.045959:FATAL:crw_web_controller.mm(3955)] Check failed: _webUsageEnabled. Tried to create a web view while suspended! Callstack: * thread #1, name = 'CrWebMain', queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0) * frame #0: 0x0000000104cb06b4 Chromium`base::debug::BreakDebugger() at debugger_posix.cc:262 frame #1: 0x0000000104d15ca8 Chromium`logging::LogMessage::~LogMessage(this=0x00007fff5c347d60) at logging.cc:783 frame #2: 0x0000000104d127c5 Chromium`logging::LogMessage::~LogMessage(this=0x00007fff5c347d60) at logging.cc:546 frame #3: 0x0000000103b8c958 Chromium`::-[CRWWebController ensureWebViewCreatedWithConfiguration:](self=0x00007fe51404bc00, _cmd="ensureWebViewCreatedWithConfiguration:", config=0x00006500001be5a0) at crw_web_controller.mm:3955 frame #4: 0x0000000103b8c7de Chromium`::-[CRWWebController ensureWebViewCreated](self=0x00007fe51404bc00, _cmd="ensureWebViewCreated") at crw_web_controller.mm:3947 frame #5: 0x0000000103b73007 Chromium`::-[CRWWebController loadCurrentURLInWebView](self=0x00007fe51404bc00, _cmd="loadCurrentURLInWebView") at crw_web_controller.mm:1662 frame #6: 0x0000000103b757eb Chromium`::-[CRWWebController loadCurrentURL](self=0x00007fe51404bc00, _cmd="loadCurrentURL") at crw_web_controller.mm:1924 frame #7: 0x0000000103b76168 Chromium`::-[CRWWebController triggerPendingLoad](self=0x00007fe51404bc00, _cmd="triggerPendingLoad") at crw_web_controller.mm:1985 frame #8: 0x0000000103b70bfd Chromium`::-[CRWWebController view](self=0x00007fe51404bc00, _cmd="view") at crw_web_controller.mm:1437 frame #9: 0x0000000103bcd9ab Chromium`web::WebStateImpl::GetView(this=0x00007fe513523ce0) at web_state_impl.mm:551 frame #10: 0x00000001046e1f31 Chromium`::-[Tab view](self=0x00007fe513523000, _cmd="view") at tab.mm:700 frame #11: 0x00000001045266d1 Chromium`::-[BrowserViewController displayTab:isNewSelection:](self=0x00007fe51b8c5000, _cmd="displayTab:isNewSelection:", tab=0x00007fe513523000, newSelection=YES) at browser_view_controller.mm:1917 frame #12: 0x000000010452954a Chromium`::-[BrowserViewController tabSelected:](self=0x00007fe51b8c5000, _cmd="tabSelected:", tab=0x00007fe513523000) at browser_view_controller.mm:2190 frame #13: 0x0000000104549c85 Chromium`::-[BrowserViewController tabModel:didChangeActiveTab:previousTab:atIndex:](self=0x00007fe51b8c5000, _cmd="tabModel:didChangeActiveTab:previousTab:atIndex:", model=0x00007fe513410ef0, newTab=0x00007fe513523000, previousTab=0x00007fe5135209c0, index=1) at browser_view_controller.mm:4584 frame #14: 0x0000000110165c6c CoreFoundation`__invoking___ + 140 frame #15: 0x0000000110165b40 CoreFoundation`-[NSInvocation invoke] + 320 frame #16: 0x000000011017d956 CoreFoundation`-[NSInvocation invokeWithTarget:] + 54 frame #17: 0x0000000104cfda3b Chromium`::-[CRBProtocolObservers forwardInvocation:](self=0x0000610000079100, _cmd="forwardInvocation:", invocation=0x00006080002683c0) at crb_protocol_observers.mm:183 frame #18: 0x0000000110164656 CoreFoundation`___forwarding___ + 534 frame #19: 0x00000001101643b8 CoreFoundation`__forwarding_prep_0___ + 120 frame #20: 0x000000010470f4a9 Chromium`::-[TabModelObserversBridge webStateList:didChangeActiveWebState:oldWebState:atIndex:userAction:](self=0x000061000002ffa0, _cmd="webStateList:didChangeActiveWebState:oldWebState:atIndex:userAction:", webStateList=0x00006100000aec40, newWebState=0x00007fe513523ce0, oldWebState=0x00007fe513520700, atIndex=1, userAction=YES) at tab_model_observers_bridge.mm:93 frame #21: 0x000000010400640c Chromium`WebStateListObserverBridge::WebStateActivatedAt(this=0x00006100002c4130, web_state_list=0x00006100000aec40, old_web_state=0x00007fe513520700, new_web_state=0x00007fe513523ce0, active_index=1, user_action=true) at web_state_list_observer_bridge.mm:111 frame #22: 0x0000000103ffc032 Chromium`WebStateList::NotifyIfActiveWebStateChanged(this=0x00006100000aec40, old_web_state=0x00007fe513520700, user_action=true) at web_state_list.mm:294 frame #23: 0x0000000103ffc67b Chromium`WebStateList::ActivateWebStateAt(this=0x00006100000aec40, index=1) at web_state_list.mm:268 frame #24: 0x00000001040083f3 Chromium`DeserializeWebStateList(web_state_list=0x00006100000aec40, session_window=0x0000608000035120, web_state_factory=0x00007fff5c34a668), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) at web_state_list_serialization.mm:187 frame #25: 0x0000000104706426 Chromium`::-[TabModel restoreSessionWindow:persistState:](self=0x00007fe513410ef0, _cmd="restoreSessionWindow:persistState:", window=0x0000608000035120, persistState=YES) at tab_model.mm:723 frame #26: 0x0000000104700981 Chromium`::-[TabModel restoreSessionWindow:](self=0x00007fe513410ef0, _cmd="restoreSessionWindow:", window=0x0000608000035120) at tab_model.mm:363 frame #27: 0x00000001046dc660 Chromium`::-[CrashRestoreHelper restoreSessionsAfterCrash](self=0x000061000004d5f0, _cmd="restoreSessionsAfterCrash") at crash_restore_helper.mm:271 frame #28: 0x00000001046dd3f8 Chromium`(anonymous namespace)::SessionCrashedInfoBarDelegate::Accept(this=0x000062800024d9e0) at crash_restore_helper.mm:160 frame #29: 0x0000000103ef37bd Chromium`InfoBarIOS::InfoBarButtonDidPress(this=0x0000628000179440, button_id=1) at infobar.mm:79 frame #30: 0x0000000103ef1d89 Chromium`::-[ConfirmInfoBarController infoBarButtonDidPress:](self=0x00006280002223c0, _cmd="infoBarButtonDidPress:", sender=0x00007fe515f49e80) at confirm_infobar_controller.mm:146 frame #31: 0x000000010e88ed22 UIKit`-[UIApplication sendAction:to:from:forEvent:] + 83 frame #32: 0x000000010ea1325c UIKit`-[UIControl sendAction:to:forEvent:] + 67 frame #33: 0x000000010ea13577 UIKit`-[UIControl _sendActionsForEvents:withEvent:] + 450 frame #34: 0x000000010ea124b2 UIKit`-[UIControl touchesEnded:withEvent:] + 618 frame #35: 0x0000000103f13634 Chromium`-[MDCButton touchesEnded:withEvent:](self=0x00007fe515f49e80, _cmd="touchesEnded:withEvent:", touches=1 element, event=0x00006100000e3e00) at MDCButton.m:331 frame #36: 0x000000010e8fc49a UIKit`-[UIWindow _sendTouchesForEvent:] + 2707 frame #37: 0x000000010e8fdbb0 UIKit`-[UIWindow sendEvent:] + 4114 frame #38: 0x000000010e8aa7b0 UIKit`-[UIApplication sendEvent:] + 352 frame #39: 0x000000010f08dadc UIKit`__dispatchPreprocessedEventFromEventQueue + 2926 frame #40: 0x000000010f085a3a UIKit`__handleEventQueue + 1122 frame #41: 0x0000000110183c01 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 frame #42: 0x00000001101690cf CoreFoundation`__CFRunLoopDoSources0 + 527 frame #43: 0x00000001101685ff CoreFoundation`__CFRunLoopRun + 911 frame #44: 0x0000000110168016 CoreFoundation`CFRunLoopRunSpecific + 406 frame #45: 0x0000000115905a24 GraphicsServices`GSEventRunModal + 62 frame #46: 0x000000010e88d0d4 UIKit`UIApplicationMain + 159 frame #47: 0x00000001038b33bb Chromium`main(argc=1, argv=0x00007fff5c34d6b0) at chrome_exe_main.mm:63 frame #48: 0x00000001180f965d libdyld.dylib`start + 1
,
May 17 2017
The bug is in chrome layer. WebState::SetWebUsageEnabled(true) has not been called hence the crash. Sylvain, I've seen that you wrote the code which inserts WebState to WebStateList, so you should know better where to call SetWebUsageEnabled(true), but from my perspective it should be inside DeserializeWebStateList. Adding M-59 RBS just in case.
,
May 17 2017
WebUsageEnabled is a property of WebState, so shouldn't it be part of the serialized state?
,
May 17 2017
I don't think we should persist WebUsageEnabled. AFAIK it is set to false only during cookie clearing to purge existing web view. So if the state is saved during cookie clearing it does not mean that WebUsageEnabled should be false. Actually I don't see any reason why WebUsageEnabled should be false for restored tab.
,
May 17 2017
I agree, it probably does not make sense to have restored WebState with WebUsageEnabled set to false. Should we fix this in WebState::CreateWithStorageSession or should I fix this in ios/chrome/?
,
May 17 2017
WebUsageEnabled is false by default because of historical (UIWebView) reasons, but making it true by default will require careful code audit and I'm not sure if this is something that we need to do for a bugfix.
,
May 18 2017
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59b25f6c12708adf7dbce3a4e08eb15ab845626b commit 59b25f6c12708adf7dbce3a4e08eb15ab845626b Author: kkhorimoto <kkhorimoto@chromium.org> Date: Fri May 19 18:13:47 2017 Add |web_usage_enabled| to DeserializeWebStateList() This allows the TabModel to pass its |webUsageEnabled_| value to the deserialized WebStateList, which prevents hitting a CHECK() in CRWWebController for attempting to trigger a load for a WebState with disabled web usage. BUG= 723665 TEST=Open Chrome, navigate to a page, quit debugger, then start up the app again and restore tabs. Should not crash. Review-Url: https://codereview.chromium.org/2894553003 Cr-Commit-Position: refs/heads/master@{#473249} [modify] https://crrev.com/59b25f6c12708adf7dbce3a4e08eb15ab845626b/ios/chrome/browser/tabs/tab_model.mm [modify] https://crrev.com/59b25f6c12708adf7dbce3a4e08eb15ab845626b/ios/chrome/browser/web_state_list/web_state_list_serialization.h [modify] https://crrev.com/59b25f6c12708adf7dbce3a4e08eb15ab845626b/ios/chrome/browser/web_state_list/web_state_list_serialization.mm [modify] https://crrev.com/59b25f6c12708adf7dbce3a4e08eb15ab845626b/ios/chrome/browser/web_state_list/web_state_list_serialization_unittest.mm [modify] https://crrev.com/59b25f6c12708adf7dbce3a4e08eb15ab845626b/ios/shared/chrome/browser/ui/browser_list/browser_list_session_service_impl.mm
,
May 19 2017
,
May 19 2017
This bug requires manual review: Less than 14 days to go before AppStore submit on M59 Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 22 2017
Checked the issue following the steps mentioned in the test line on M60.0.3107.0 canary on iPhone 6s with iOS 10.3.1 and iPad Air with iOS 10.2.1 Open Chrome, navigate to a page, quit debugger, (about://inducebrowsercrashforrealz to crash the app) then start up the app again and restore tabs. There are no crashes observed. Looks good.
,
May 23 2017
,
May 24 2017
Merging this will also require merging https://codereview.chromium.org/2808773004/, since my CL is dependent on that one. That CL is on the larger side, so requesting permission once again.
,
May 24 2017
This bug requires manual review: Less than 9 days to go before AppStore submit on M59 Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 24 2017
Hey Kurt please confirm back here after merging that this works as expected.
,
May 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da2e896aee0b0b634c064cb3ad2ed4d71a38349e commit da2e896aee0b0b634c064cb3ad2ed4d71a38349e Author: kkhorimoto <kkhorimoto@chromium.org> Date: Wed May 24 21:02:53 2017 [ios] Change WebStateList serialization method to use SessionWindowIOS. In order to properly save and restore the application state in the new architecture, (partially) move the implementation outside of TabModel (which is not used in the new architecture). BUG= 710848 , 723665 NOTRY=true NOPRESUBMIT=true TBR=sdefresne@chromium.org Review-Url: https://codereview.chromium.org/2808773004 Cr-Original-Commit-Position: refs/heads/master@{#465969} Review-Url: https://codereview.chromium.org/2897333002 Cr-Commit-Position: refs/branch-heads/3071@{#686} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/browsing_data/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/crash_report/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/sessions/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/sessions/NSCoder+Compatibility.mm [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/sessions/session_util.mm [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/tabs/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/tabs/tab_model.mm [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/test/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/ui/main/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/web_state_list/BUILD.gn [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/web_state_list/web_state_list_serialization.h [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/web_state_list/web_state_list_serialization.mm [modify] https://crrev.com/da2e896aee0b0b634c064cb3ad2ed4d71a38349e/ios/chrome/browser/web_state_list/web_state_list_serialization_unittest.mm
,
May 24 2017
Cherry-picked and verified that tabs can be restored after a crash/force quite without issue.
,
May 31 2017
Verified the issue following the steps mentioned in the test line of comment#8 on 59.0.3071.82 beta on iPhone 6 with iOS 10.3.1 and iPad Air with iOS 10.2.1 Open Chrome, navigate to a page "https://mhahmadi.github.io/pr-demo/" quit debugger, (about://inducebrowsercrashforrealz to crash the app) then start up the app again and restore tabs. There are no crashes observed. Looks good. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by sdefresne@chromium.org
, May 17 2017