New issue
Advanced search Search tips

Issue 747121 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

[iOS] Settings view reloaded after being dismissed

Project Member Reported by jlebel@chromium.org, Jul 20 2017

Issue description

If the sign-in interaction controller is dismissed by opening an URL, the setting view controller is dismissed, which canceled the sign-in interaction controller.
When the sign-in interaction controller is cancelled, completion callback from the setting view controller is called, which triggers a reload of the setting view controller, and then a new sign-in promo view mediator (in the reload method).
For this mediator, the method -[SigninPromoViewMediator signinPromoViewRemoved] is never called.

* thread #1, name = 'CrWebMain', queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000100c66e26 ios_chrome_ui_egtests`::-[SettingsCollectionViewController loadModel](self=0x00007f826e11b000, _cmd="loadModel") at settings_collection_view_controller.mm:349
    frame #1: 0x0000000100c794d8 ios_chrome_ui_egtests`::-[SettingsRootCollectionViewController reloadData](self=0x00007f826e11b000, _cmd="reloadData") at settings_root_collection_view_controller.mm:117
    frame #2: 0x0000000100c6dc27 ios_chrome_ui_egtests`::-[SettingsCollectionViewController didFinishSignin:](self=0x00007f826e11b000, _cmd="didFinishSignin:", signedIn=NO) at settings_collection_view_controller.mm:1011
    frame #3: 0x0000000100c6db57 ios_chrome_ui_egtests`::__71-[SettingsCollectionViewController showSignInWithIdentity:promoAction:]_block_invoke(.block_descriptor=<unavailable>, success=NO) at settings_collection_view_controller.mm:1003
    frame #4: 0x0000000100e3ebc6 ios_chrome_ui_egtests`::-[SigninInteractionController runCompletionCallbackWithSuccess:showAccountsSettings:](self=0x00006080000b18e0, _cmd="runCompletionCallbackWithSuccess:showAccountsSettings:", success=NO, showAccountsSettings=NO) at signin_interaction_controller.mm:399
    frame #5: 0x0000000100e3da26 ios_chrome_ui_egtests`::-[SigninInteractionController dismissSigninViewControllerWithSignInSuccess:showAccountsSettings:](self=0x00006080000b18e0, _cmd="dismissSigninViewControllerWithSignInSuccess:showAccountsSettings:", success=NO, showAccountsSettings=NO) at signin_interaction_controller.mm:319
    frame #6: 0x0000000100e3e86e ios_chrome_ui_egtests`::-[SigninInteractionController didFailSignIn:](self=0x00006080000b18e0, _cmd="didFailSignIn:", controller=0x00007f826d5b9a90) at signin_interaction_controller.mm:371
    frame #7: 0x0000000100e20e69 ios_chrome_ui_egtests`::-[ChromeSigninViewController cancel](self=0x00007f826d5b9a90, _cmd="cancel") at chrome_signin_view_controller.mm:244
    frame #8: 0x0000000100e3a7bd ios_chrome_ui_egtests`::-[SigninInteractionController cancel](self=0x00006080000b18e0, _cmd="cancel") at signin_interaction_controller.mm:100
    frame #9: 0x0000000100c6dfc5 ios_chrome_ui_egtests`::-[SettingsCollectionViewController settingsWillBeDismissed](self=0x00007f826e11b000, _cmd="settingsWillBeDismissed") at settings_collection_view_controller.mm:1050
    frame #10: 0x0000000100c74279 ios_chrome_ui_egtests`::-[SettingsNavigationController settingsWillBeDismissed](self=0x00007f827002ca00, _cmd="settingsWillBeDismissed") at settings_navigation_controller.mm:310
    frame #11: 0x00000001008dd9f4 ios_chrome_ui_egtests`::-[MainController closeSettingsAnimated:completion:](self=0x00007f826d607130, _cmd="closeSettingsAnimated:completion:", animated=NO, completion=0x00000001008dfa40) at main_controller.mm:2140
    frame #12: 0x00000001008df81f ios_chrome_ui_egtests`::-[MainController dismissModalDialogsWithCompletion:](self=0x00007f826d607130, _cmd="dismissModalDialogsWithCompletion:", completion=0x00000001008e0ba0) at main_controller.mm:2320
    frame #13: 0x00000001008e0a9c ios_chrome_ui_egtests`::-[MainController dismissModalsAndOpenSelectedTabInMode:withURL:transition:completion:](self=0x00007f826d607130, _cmd="dismissModalsAndOpenSelectedTabInMode:withURL:transition:completion:", targetMode=NORMAL, url=0x00007f826fa04b30, transition=PAGE_TRANSITION_TYPED, handler=(null)) at main_controller.mm:2395
    frame #14: 0x00000001008d60e0 ios_chrome_ui_egtests`::-[MainController openUrl:](self=0x00007f826d607130, _cmd="openUrl:", command=0x00007f826fa04b20) at main_controller.mm:1495
    frame #15: 0x00000001008d4bbc ios_chrome_ui_egtests`::-[MainController chromeExecuteCommand:](self=0x00007f826d607130, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) at main_controller.mm:1383
    frame #16: 0x00000001008cc657 ios_chrome_ui_egtests`::-[MainApplicationDelegate chromeExecuteCommand:](self=0x0000600000285640, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) at main_application_delegate.mm:257
    frame #17: 0x0000000100828095 ios_chrome_ui_egtests`::-[UIWindow(self=0x00007f826d40c6d0, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) chromeExecuteCommand:](id) at UIKit+ChromeExecuteCommand.mm:25
    frame #18: 0x0000000100827f58 ios_chrome_ui_egtests`::-[UIResponder(self=0x00007f826d6bd5d0, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) chromeExecuteCommand:](id) at UIKit+ChromeExecuteCommand.mm:14
    frame #19: 0x0000000100827f58 ios_chrome_ui_egtests`::-[UIResponder(self=0x00007f826d527880, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) chromeExecuteCommand:](id) at UIKit+ChromeExecuteCommand.mm:14
    frame #20: 0x0000000100827f58 ios_chrome_ui_egtests`::-[UIResponder(self=0x00007f826f81fd40, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) chromeExecuteCommand:](id) at UIKit+ChromeExecuteCommand.mm:14
    frame #21: 0x0000000100827f58 ios_chrome_ui_egtests`::-[UIResponder(self=0x00007f8270079600, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) chromeExecuteCommand:](id) at UIKit+ChromeExecuteCommand.mm:14
    frame #22: 0x0000000101355b4a ios_chrome_ui_egtests`::-[BrowserViewController chromeExecuteCommand:](self=0x00007f8270079600, _cmd="chromeExecuteCommand:", sender=0x00007f826fa04b20) at browser_view_controller.mm:4210
    frame #23: 0x00000001017d6109 ios_chrome_ui_egtests`chrome_test_util::RunCommandWithActiveViewController(command=0x00007f826fa04b20) at chrome_test_util.mm:157
    frame #24: 0x000000010187aeb0 ios_chrome_ui_egtests`::-[SigninInteractionControllerTestCase testSignInCancelAddAccount](self=0x0000618000442160, _cmd="testSignInCancelAddAccount") at signin_interaction_controller_egtest.mm:494
    frame #25: 0x000000010c0c2c6c CoreFoundation`__invoking___ + 140
    frame #26: 0x000000010c0c2b40 CoreFoundation`-[NSInvocation invoke] + 320
    frame #27: 0x00000001108cb8d8 EarlGrey`-[GREYTestCaseInvocation invoke](self=0x0000618000663300, _cmd="invoke") at GREYTestCaseInvocation.m:48
    frame #28: 0x000000010d36e8e2 XCTest`__24-[XCTestCase invokeTest]_block_invoke_2 + 491
    frame #29: 0x000000010d3acf62 XCTest`-[XCTestContext performInScope:] + 190
    frame #30: 0x000000010d36e6e4 XCTest`-[XCTestCase invokeTest] + 254
    frame #31: 0x00000001108bc804 EarlGrey`-[XCTestCase(self=0x0000618000442160, _cmd="invokeTest") grey_invokeTest] at XCTestCase+GREYAdditions.m:217
    frame #32: 0x000000010d36efb2 XCTest`-[XCTestCase performTest:] + 565
    frame #33: 0x000000010d36c078 XCTest`__27-[XCTestSuite performTest:]_block_invoke + 300
    frame #34: 0x000000010d36bc68 XCTest`-[XCTestSuite _performProtectedSectionForTest:testSection:] + 29
    frame #35: 0x000000010d36be4e XCTest`-[XCTestSuite performTest:] + 214
    frame #36: 0x000000010d36c078 XCTest`__27-[XCTestSuite performTest:]_block_invoke + 300
    frame #37: 0x000000010d36bc68 XCTest`-[XCTestSuite _performProtectedSectionForTest:testSection:] + 29
    frame #38: 0x000000010d36be4e XCTest`-[XCTestSuite performTest:] + 214
    frame #39: 0x000000010d36c078 XCTest`__27-[XCTestSuite performTest:]_block_invoke + 300
    frame #40: 0x000000010d36bc68 XCTest`-[XCTestSuite _performProtectedSectionForTest:testSection:] + 29
    frame #41: 0x000000010d36be4e XCTest`-[XCTestSuite performTest:] + 214
    frame #42: 0x000000010d3b8a86 XCTest`__44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke + 40
    frame #43: 0x000000010d37af27 XCTest`-[XCTestObservationCenter _observeTestExecutionForBlock:] + 574
    frame #44: 0x000000010d3b8925 XCTest`-[XCTTestRunSession runTestsAndReturnError:] + 281
    frame #45: 0x000000010d356a50 XCTest`-[XCTestDriver runTestsAndReturnError:] + 254
    frame #46: 0x000000010d3b086e XCTest`_XCTestMain + 559
    frame #47: 0x000000010c0e0b5c CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
    frame #48: 0x000000010c0c5e54 CoreFoundation`__CFRunLoopDoBlocks + 356
    frame #49: 0x000000010c0c55ee CoreFoundation`__CFRunLoopRun + 894
    frame #50: 0x000000010c0c5016 CoreFoundation`CFRunLoopRunSpecific + 406
    frame #51: 0x0000000113402a24 GraphicsServices`GSEventRunModal + 62
    frame #52: 0x000000010d84e134 UIKit`UIApplicationMain + 159
    frame #53: 0x000000010012f6a3 ios_chrome_ui_egtests`(anonymous namespace)::RunUIApplicationMain(argc=5, argv=0x00007fff5fad12a8) at chrome_exe_main.mm:51
    frame #54: 0x000000010012f19e ios_chrome_ui_egtests`main(argc=5, argv=0x00007fff5fad12a8) at chrome_exe_main.mm:74
    frame #55: 0x0000000115b4365d libdyld.dylib`start + 1
    frame #56: 0x0000000115b4365d libdyld.dylib`start + 1

 

Comment 1 by jlebel@chromium.org, Jul 20 2017

Labels: -M-60 M-61

Comment 2 by jlebel@chromium.org, Jul 20 2017

Relevant stack trace:
stack trace:
-[SettingsCollectionViewController loadModel]
...
-[SettingsCollectionViewController didFinishSignin:]
...
-[SigninInteractionController cancel]
-[SettingsCollectionViewController settingsWillBeDismissed]


Project Member

Comment 3 by bugdroid1@chromium.org, Jul 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5ed82db1604d7b1badd71431853ce36ac9d3f7a

commit e5ed82db1604d7b1badd71431853ce36ac9d3f7a
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Fri Jul 21 09:30:09 2017

Don't reload setting view controller if it has been dismissed

If the setting view controller is being reloaded while being dismissed,
a new SigninPromoViewMediator is created. This new one is not properly
released. -[SigninPromoViewMediator signinPromoViewRemoved] has to be
called before dealloc.

stack trace:
-[SettingsCollectionViewController loadModel]
...
-[SettingsCollectionViewController didFinishSignin:]
...
-[SigninInteractionController cancel]
-[SettingsCollectionViewController settingsWillBeDismissed]
...


Bug introduced with crrev.com/2883093002
DCHECK introduced with crrev.com/c/567923

Bug:  747121 
Change-Id: Ida6847e0942c5145ddc8fbd6c4681bfcc87a8904
Reviewed-on: https://chromium-review.googlesource.com/580408
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488624}
[modify] https://crrev.com/e5ed82db1604d7b1badd71431853ce36ac9d3f7a/ios/chrome/browser/ui/settings/settings_collection_view_controller.mm
[modify] https://crrev.com/e5ed82db1604d7b1badd71431853ce36ac9d3f7a/ios/chrome/browser/ui/settings/settings_navigation_controller_unittest.mm

Comment 4 by jlebel@chromium.org, Jul 21 2017

Status: Fixed (was: Started)

Comment 5 by jlebel@chromium.org, Jul 21 2017

Labels: Merge-Request-61
Project Member

Comment 6 by sheriffbot@chromium.org, Jul 22 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 25 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69011449523ebbed981715ef3a4a4aa24097bbcb

commit 69011449523ebbed981715ef3a4a4aa24097bbcb
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Tue Jul 25 13:05:00 2017

Don't reload setting view controller if it has been dismissed

If the setting view controller is being reloaded while being dismissed,
a new SigninPromoViewMediator is created. This new one is not properly
released. -[SigninPromoViewMediator signinPromoViewRemoved] has to be
called before dealloc.

stack trace:
-[SettingsCollectionViewController loadModel]
...
-[SettingsCollectionViewController didFinishSignin:]
...
-[SigninInteractionController cancel]
-[SettingsCollectionViewController settingsWillBeDismissed]
...

Bug introduced with crrev.com/2883093002
DCHECK introduced with crrev.com/c/567923

TBR=jlebel@chromium.org

(cherry picked from commit e5ed82db1604d7b1badd71431853ce36ac9d3f7a)

Bug:  747121 
Change-Id: Ida6847e0942c5145ddc8fbd6c4681bfcc87a8904
Reviewed-on: https://chromium-review.googlesource.com/580408
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#488624}
Reviewed-on: https://chromium-review.googlesource.com/582827
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#23}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/69011449523ebbed981715ef3a4a4aa24097bbcb/ios/chrome/browser/ui/settings/settings_collection_view_controller.mm
[modify] https://crrev.com/69011449523ebbed981715ef3a4a4aa24097bbcb/ios/chrome/browser/ui/settings/settings_navigation_controller_unittest.mm

Sign in to add a comment