1207671 - Crash on top-site removal

Status: RESOLVED WORKSFORME

(Firefox for iOS :: Home screen, defect)

Description

[Aaron Train [:aaronmt]]

Hit this crash deleting top-sites on master, I had 5 top-sites alongside the default, I deleted the 5th


2015-09-23 12:49:14.059 Client[1318:2024837] *** Assertion failure in -[_TtC6ClientP33_C9DFE36B31E426C19626C23F17C1D22922TopSitesCollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:], /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIKit/UIKit-3505.16/UICollectionView.m:4211

2015-09-23 12:49:14.075 Client[1318:2024837] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of items in section 0.  The number of items contained in an existing section after the update (7) must be equal to the number of items contained in that section before the update (7), plus or minus the number of items inserted or deleted from that section (0 inserted, 1 deleted) and plus or minus the number of items moved into or out of that section (0 moved in, 0 moved out).'


* thread #1: tid = 0x1ee585, 0x00000001996e71e0 libsystem_kernel.dylib`__pthread_kill + 8, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00000001996e71e0 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x00000001997b0f0c libsystem_pthread.dylib`pthread_kill + 112
    frame #2: 0x000000019965ab78 libsystem_c.dylib`abort + 140
    frame #3: 0x000000019841d3f4 libc++abi.dylib`abort_message + 132
    frame #4: 0x0000000198439e98 libc++abi.dylib`default_terminate_handler() + 304
    frame #5: 0x0000000198da0248 libobjc.A.dylib`_objc_terminate() + 124
    frame #6: 0x0000000198436f44 libc++abi.dylib`std::__terminate(void (*)()) + 16
    frame #7: 0x000000019843685c libc++abi.dylib`__cxa_throw + 136
    frame #8: 0x0000000198da0094 libobjc.A.dylib`objc_exception_throw + 332
    frame #9: 0x00000001841ace2c CoreFoundation`+[NSException raise:format:arguments:] + 108
    frame #10: 0x000000018509bf3c Foundation`-[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 112
    frame #11: 0x0000000189ee9a30 UIKit`-[UICollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:] + 12248
    frame #12: 0x0000000189eeccc0 UIKit`-[UICollectionView _performBatchUpdates:completion:invalidationContext:tentativelyForReordering:] + 360
    frame #13: 0x0000000189eecb3c UIKit`-[UICollectionView _performBatchUpdates:completion:invalidationContext:] + 84
    frame #14: 0x00000001898ebb38 UIKit`-[UICollectionView performBatchUpdates:completion:] + 64
  * frame #15: 0x000000010019be24 Client`Client.TopSitesPanel.(result=Success, indexPath=0xc000000000800016, self=0x00000001404f20c0) (Client.TopSitesPanel)(Shared.Maybe<Storage.Cursor<Storage.Site>>, indexPath : ObjectiveC.NSIndexPath) -> () + 1296 at TopSitesPanel.swift:156
    frame #16: 0x00000001001a4778 Client`Client.TopSitesPanel.(result=Failure, self=0x00000001404f20c0, indexPath=0xc000000000800016) (Client.TopSitesPanel) -> (Storage.Site, atIndexPath : ObjectiveC.NSIndexPath) -> ()).(closure #1).(closure #1) + 492 at TopSitesPanel.swift:128
    frame #17: 0x00000001009985b8 Shared`reabstraction thunk helper <A> from @callee_owned (@in A) -> (@unowned ()) to @callee_owned (@in A) -> (@out ()) + 32 at DeferredUtils.swift:0
    frame #18: 0x00000001009cec80 Shared`reabstraction thunk helper <A> from @callee_owned (@in A) -> (@out ()) to @callee_owned (@in A) -> (@unowned ()) + 32 at Deferred.swift:0
    frame #19: 0x00000001009cecf0 Shared`Shared.Deferred.(block=(Shared`partial apply forwarder for reabstraction thunk helper <A> from @callee_owned (@in A) -> (@out ()) to @callee_owned (@in A) -> (@unowned ()) with unmangled suffix "35" at Deferred.swift), filledValue=Failure) <A> (Shared.Deferred<A>) -> (A, assertIfFilled : Swift.Bool) -> ()).(closure #2) + 100 at Deferred.swift:44
    frame #20: 0x0000000100986e34 Shared`reabstraction thunk helper from @callee_owned () -> (@unowned ()) to @callee_unowned @convention(block) () -> (@unowned ()) + 44 at ReadWriteLock.swift:0
    frame #21: 0x0000000102515d70 libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame #22: 0x0000000102515d30 libdispatch.dylib`_dispatch_client_callout + 16
    frame #23: 0x000000010251b780 libdispatch.dylib`_dispatch_main_queue_callback_4CF + 2096
    frame #24: 0x0000000184164258 CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12
    frame #25: 0x00000001841620c0 CoreFoundation`__CFRunLoopRun + 1628
    frame #26: 0x0000000184090dc0 CoreFoundation`CFRunLoopRunSpecific + 384
    frame #27: 0x000000018f1e4088 GraphicsServices`GSEventRunModal + 180
    frame #28: 0x000000018976af60 UIKit`UIApplicationMain + 204
    frame #29: 0x000000010027faa0 Client`main + 528 at main.swift:22
    frame #30: 0x00000001995ca8b8 libdyld.dylib`start + 4

Comment 1

[Aaron Train [:aaronmt]]

Crashed in

    private func deleteHistoryTileForSite(site: Site, atIndexPath indexPath: NSIndexPath) {
        profile.history.removeSiteFromTopSites(site) >>== {
            self.profile.history.getSitesByFrecencyWithLimit(self.layout.thumbnailCount).uponQueue(dispatch_get_main_queue(), block: { result in
                self.updateDataSourceWithSites(result)
                self.deleteOrUpdateSites(result, indexPath: indexPath)
            })
        }
    }

Comment 2

[Aaron Train [:aaronmt]]

Reproducible on Test Flight as well

Comment 3

[Richard Newman [:rnewman]]

My diagnosis:

The UICollectionView thinks that this is a deletion. It has invariants, such as "if the collection has N items, and you delete 1, it'll have N-1 items".

That's not true for top sites as implemented. If you hide a domain from top sites, when we requery you'll get the same limited number of results, with the removed domain gone but the next-highest-ranked domain added at the end of the list.

(The invariant will hold if you have a very small number of visited domains, which is presumably why this didn't come up in testing.)


We have three choices here.

1. Don't treat this as a deletion as understood by UICollectionView; remove or suppress the invariant, or take a different code path.
2. Fetch one fewer item when requerying. This is kinda weird.
3. Don't requery: remove the tile instead. It'll reappear with another item next time the user visits Top Sites, so might also look a little strange.


None of this is storage related, so moving this to Home Panels.

Comment 4

[Emily Toop (:fluffyemily)]

I cannot replicate this at all. If anyone can come up with a reproducible STR then happy to revisit, but there is nothing obvious in the code that would lead to this.