1217843 - Firefox crashes after a "Clear private data" action is performed

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect)

Description

[u549602]

Device: iPod 5
iOS firmware : 8.4

Steps to reproduce:

1. Visits some sites in order to produce some navigation data
2. Go to settings - Clear private data
3. Clear private data - back - Done
4. Close already opened tabs

Result:
After the user closes the first opened tab, the app crashes 


Please note that on an iPod Touch 5 with 9.0.2 firmware the issue could not be reproduced.

Comment 1

[Catalin Suciu [:csuciu]]

Attachment: Client 23-10-15 16-48.crash

Comment 2

[Stephan Leroux [:sleroux]]

Attachment: Client 2015-10-23, 9-48 AM.crash

Symbolicated

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

NSFoundationVersionNumber_iOS_8_3 :-(

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

13:56:35       AaronMT | sleroux: st3fan: so what's the story with the CPD bug
13:57:47       sleroux | AaronMT: I was going to investigate further this afternoon but it looks like it's a bug with a codepath that only effects <= 8.4 devices

No I think the problem is that the code within that version check is supposed to execute if we do not run on iOS 9.

But, we wrote it when 8.3 was the most recent so the check we do is to see if the OS version is <= NSFoundationVersionNumber_iOS_8_3, which simply misses anything in between 8.3 and 9.0. Aka, 8.4.

(8.4 is the OS version in the crash report)

Comment 5

[Stephan Leroux [:sleroux]]

Attachment: https://github.com/mozilla/firefox-ios/pull/1192

So there actually isn't a flag for 8_4. The highest version that NSFoundationVersionNumber offers is 8_3. I think it might be misleading because it starts at 8_0 so 8_3 might really mean 8.4?

Anyways, I did find a pretty significant issue with the Tabs Tray and why the app was crashing. Clearing private data is the only place in the app which closes tabs on the Tab Tray without interacting with the UI. As part of the PB Mode work, I added some logic to didRemoveTab that relies on selected indexPath of the item that was deleted [1]. Since the deletion from CPD is not triggered from UI, this code path never happens which makes the tabDataSource.tabs list and tabManager out of sync. It also crashes in the didAddTab callback on debug builds because this assert is thrown [2]. Removing this removedIndexPath dependency and only use the tab to find out what to delete resolves this issue.

Also, the reason this crashes on iOS < 9 is because the codepath for iOS 9 doesn't remove tabs from the tab tray.

[1] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Browser/TabTrayController.swift#L568

[2] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Browser/TabTrayController.swift#L558 & https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Browser/TabManager.swift#L163

Comment 6

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8678855 [details] [review]
https://github.com/mozilla/firefox-ios/pull/1192

LGTM

Comment 7

[Stephan Leroux [:sleroux]]

Merged

Comment 8

[u549602]

Verified as fixed on Version 1.2(1201) by using Ipod Touch 5 with 8.4 firmware.