New issue
Advanced search Search tips

Issue 795895 link

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



Sign in to add a comment

DCHECK failure in Other Devices tab while using the sign-in promo

Project Member Reported by jlebel@chromium.org, Dec 18 2017 
Install Chrome
Don't sign-in
Open the "Other Devices" tab
Sign-in with "Continue as ..."

Result, in -[SigninPromoViewMediator signinPromoViewRemoved], this DCHECK fails:
  DCHECK(!self.signinInProgress);

(lldb) bt
* thread #1, name = 'CrWebMain', queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0)
  * frame #0: 0x00000001027c8944 Chromium`base::debug::BreakDebugger() at debugger_posix.cc:269
    frame #1: 0x000000010282bfd9 Chromium`logging::LogMessage::~LogMessage(this=0x00007fff5ec0aab8) at logging.cc:844
    frame #2: 0x00000001028286f5 Chromium`logging::LogMessage::~LogMessage(this=0x00007fff5ec0aab8) at logging.cc:575
    frame #3: 0x0000000101d4104e Chromium`::-[SigninPromoViewMediator signinPromoViewRemoved](self=0x000062c000280dc0, _cmd="signinPromoViewRemoved") at signin_promo_view_mediator.mm:475
    frame #4: 0x000000010256d1df Chromium`::-[TabSwitcherPanelOverlayView dealloc](self=0x00007f874d52fff0, _cmd="dealloc") at tab_switcher_panel_overlay_view.mm:196
    frame #5: 0x0000000113d73178 libobjc.A.dylib`(anonymous namespace)::AutoreleasePoolPage::pop(void*) + 860
    frame #6: 0x000000010e851536 CoreFoundation`_CFAutoreleasePoolPop + 22
    frame #7: 0x000000010c6898ac UIKit`_prepareForCAFlush + 143
    frame #8: 0x000000010c6b9c8f UIKit`_afterCACommitHandler + 252
    frame #9: 0x000000010e8a9d37 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    frame #10: 0x000000010e8a9c8e CoreFoundation`__CFRunLoopDoObservers + 430
    frame #11: 0x000000010e88e254 CoreFoundation`__CFRunLoopRun + 1572
    frame #12: 0x000000010e88d9b9 CoreFoundation`CFRunLoopRunSpecific + 409
    frame #13: 0x00000001152329c6 GraphicsServices`GSEventRunModal + 62
    frame #14: 0x000000010c68f5e8 UIKit`UIApplicationMain + 159
    frame #15: 0x0000000100ff4d32 Chromium`(anonymous namespace)::RunUIApplicationMain(argc=1, argv=0x00007fff5ec0c548) at chrome_exe_main.mm:55
    frame #16: 0x0000000100ff4823 Chromium`main(argc=1, argv=0x00007fff5ec0c548) at chrome_exe_main.mm:79
    frame #17: 0x0000000116737d81 libdyld.dylib`start + 1
    frame #18: 0x0000000116737d81 libdyld.dylib`start + 1
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 20 2017 

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

commit e273042d34106a89730915faea6bc4f8e0e103bc
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Wed Dec 20 11:27:19 2017

[iOS][Signin] Fixing DCHECK and UI glitch in Other Devices tab

When the user taps on the "Continue as ..." button in the sign-in
promo, of the Other Devices tab, the user is signed in at the
beginning of the sync confirmation animation.
Therefore the TabSwitcherModel receives the sign-in notification and
send a notification to the TabSwitcherController to reload before the
end of the animation. While reloading the TabSwitcherController
removes the current TabSwitcherPanelOverlayView that contains the
sign-in promo view and its SigninPromoViewMediator.
The issue is that SigninPromoViewMediator is supposed to live until
the end of the sign-in to handle correctly the metrics (histograms
and user actions).

To avoid that, the SigninPromoViewMediator is now owned by the
TabSwitcherController. And TabSwitcherModel asks its delegate if a
sign-in is in progress. In that case, the sign-in notification is
ignored.
Therefore the UI is not updated while the sign-in interaction is
appearing (see  crbug.com/725925 )

Once the sign-in is finished (cancel or success), the
TabSwitcherController is notified by SigninPromoViewMediator and then
the TabSwitcherModel can be reloaded.

Bug:  795895 ,  725925 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Iefff5975bc251ea39cb06c5d73a4adc135ce317b
Reviewed-on: https://chromium-review.googlesource.com/832391
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525309}
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_controller.mm
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_controller_egtest.mm
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_model.h
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_model.mm
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_model_unittest.mm
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_overlay_view.h
[modify] https://crrev.com/e273042d34106a89730915faea6bc4f8e0e103bc/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_overlay_view.mm
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 21 2017 

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

commit 0e708e7b0bcc70258175ef83de2180e589dbd022
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Thu Dec 21 17:01:38 2017

[iOS][Signin] Cleanup in TabSwitcherPanelOverlayView

This patch is related to crrev.com/c/832391.
The goal is to remove the knowledge of SigninPromoViewMediator from
TabSwitcherPanelOverlayView.

Bug:  795895 ,  725925 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4ef0ee97d0f4adf91c40f5676ad53b2e53700fe5
Reviewed-on: https://chromium-review.googlesource.com/836613
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525717}
[modify] https://crrev.com/0e708e7b0bcc70258175ef83de2180e589dbd022/ios/chrome/browser/ui/tab_switcher/tab_switcher_controller.mm
[modify] https://crrev.com/0e708e7b0bcc70258175ef83de2180e589dbd022/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_overlay_view.h
[modify] https://crrev.com/0e708e7b0bcc70258175ef83de2180e589dbd022/ios/chrome/browser/ui/tab_switcher/tab_switcher_panel_overlay_view.mm

Comment 3 by jlebel@chromium.org, Dec 21 2017

Status: Fixed (was: Started)
►

Sign in to add a comment