New issue
Advanced search Search tips

Issue 890332 link

Starred by 1 user
Status: Fixed
Owner:
jlebel@chromium.org
Closed: Oct 15
Cc:
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

DCHECK while switching sync from a managed account to another manage account

Project Member Reported by jlebel@chromium.org, Sep 28 
This bug is to track:  crbug.com/890312#c1 :

While testing it, I was able to hit that DCHECK [1] But this is not a crasher.

To leave the managed account, an alert dialog is created [2]. And in the callback, the delegate is called first [3], and then the dialog is cleanup in the following line (setting _alertCoordinator to nil).
The issue is the delegate calls -[AuthenticationFlowPerformer showManagedConfirmationForHostedDomain:viewController:] which creates a new alert dialog and stores it in _alertCoordinator, and this is done before the cleanup of the first alert dialog. So DCHECK [1] fails.

The consequence is _alertCoordinator is set to nil while the dialog box is opened. This should not be a crasher since the alert is being displayed.
This issue happens for all alert dialog in that file.

[1] https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm?l=321
[2] https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm?l=205
[3] https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm?l=216
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 28 

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

commit 4e2538c029a3d7086933f173249e935f73174d24
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Fri Sep 28 14:43:44 2018

[iOS] Fixing DCHECK while switching sync accounts

The alert dialog property should be set to nil in the dialog callback
before doing anything else.
DCHECK is failing since the first alert dialog still exist in
_alertCoordinator, when creating the second alert dialog:
https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm?l=321

There is no consequence since when cleaning up _alertCoordinator for the
first dialog (while the second dialog is already set into _alertCoordinator).
_alertCoordinator is not set to nil because of that test [1] when the
cleanup is done [2].

[1]: https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm?l=387
[2]: https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm?l=218


Bug:  890332 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I012baa44c3a129a54bc8a3d4c0f442b06e04b68a
Reviewed-on: https://chromium-review.googlesource.com/1251601
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595087}
[modify] https://crrev.com/4e2538c029a3d7086933f173249e935f73174d24/ios/chrome/browser/ui/authentication/authentication_flow_performer.mm

Comment 2 by jlebel@chromium.org, Oct 15

Status: Fixed (was: Available)
►

Sign in to add a comment