1400883 - Crash: Pre-verification screen crashes after sending NotificationFirefoxAccountProfileChanged.

Status: RESOLVED FIXED

(Firefox for iOS :: Firefox Accounts, defect, P1)

Description

[James Hugman [:jhugman] [@jhugman]]

STR: 

1. Login to FxA with an account that you have not logged into from this IP address today.

Expected results:
Confirmation page appears.

Observed results:
Confirmation page appears and app immediately crashes.


-- 

This appears to be a problem with NotificationFirefoxAccountProfileChanged notifications been received when the Settings screen is offscreen.


2017-09-18 15:36:44.645 Client[9267:20669779] *** Assertion failure in -[UITableView _endCellAnimationsWithContext:], /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIKit_Sim/UIKit-3600.7.47/UITableView.m:1737
2017-09-18 15:36:44.651 Client[9267:20669779] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of rows in section 0.  The number of rows contained in an existing section after the update (2) must be equal to the number of rows contained in that section before the update (1), plus or minus the number of rows inserted or deleted from that section (1 inserted, 1 deleted) and plus or minus the number of rows moved into or out of that section (0 moved in, 0 moved out).'
*** First throw call stack:
(
	0   CoreFoundation                      0x000000010d5ccb0b __exceptionPreprocess + 171
	1   libobjc.A.dylib                     0x000000010c67e141 objc_exception_throw + 48
	2   CoreFoundation                      0x000000010d5d0cf2 +[NSException raise:format:arguments:] + 98
	3   Foundation                          0x000000010c218536 -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 193
	4   UIKit                               0x000000010e84ac38 -[UITableView _endCellAnimationsWithContext:] + 16362
	5   UIKit                               0x000000010e861d1c -[UITableView _updateRowsAtIndexPaths:updateAction:withRowAnimation:] + 329
	6   Client                              0x0000000108b20121 _T06Client20AccountStatusSettingC06updateB0y10Foundation12NotificationV12notification_tFyycfU_ + 369
	7   Client                              0x0000000108b36e82 _T06Client20AccountStatusSettingC06updateB0y10Foundation12NotificationV12notification_tFyycfU_TA + 50
	8   Client                              0x0000000108b0d059 _T0Ix_IyB_TR + 41
	9   libdispatch.dylib                   0x00000001117524a6 _dispatch_call_block_and_release + 12
	10  libdispatch.dylib                   0x000000011177b05c _dispatch_client_callout + 8
	11  libdispatch.dylib                   0x000000011175c40b _dispatch_main_queue_callback_4CF + 411
	12  CoreFoundation                      0x000000010d591909 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
	13  CoreFoundation                      0x000000010d557ae4 __CFRunLoopRun + 2164
	14  CoreFoundation                      0x000000010d557016 CFRunLoopRunSpecific + 406
	15  GraphicsServices                    0x0000000113514a24 GSEventRunModal + 62
	16  UIKit                               0x000000010e70c134 UIApplicationMain + 159
	17  Client                              0x0000000108ef6535 main + 661
	18  libdyld.dylib                       0x00000001117c765d start + 1
)

Comment 1

[Vijay Budhram]

Hey James,

I was not able to reproduce this error. I tested from the current master branch.

  * Forced sign-in confirmation by using an @mozilla.com email address on stage
  * Tested iOS 11 and iOS 10
  * Tested from Onboarding flow and settings sign-in

Is there another step that I might be missing?

Comment 2

[Justin D'Arcangelo [:justindarc]]

James, what type of device/iOS version are you using to reproduce this?

Comment 3

[James Hugman [:jhugman] [@jhugman]]

Vijay — this bug is exposed by:

https://github.com/mozilla-mobile/firefox-ios/pull/3177

The updateAccount causes a postNotification to happen; 

1. this shouldn't happen from a deinit block
2. it's unclear what state the setting screen is doing at that point.

Comment 4

[Vijay Budhram]

Attachment: Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/3204

Hey James, this moves the profile update into the viewDidDisappear function, which I believe will resolve the crash. r?

Comment 5

[James Hugman [:jhugman] [@jhugman]]

Comment on attachment 8910785 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/3204

Looks good. Please land ASAP.

Also: please could you file a bug to help clean up observers in the settings screen. The root of this bug is that the NotificationFirefoxAccountProfileChanged shouldn't be being listened to while settings is not on screen.

Comment 6

[Vijay Budhram]

Merged to master