Wordpress-ios: WizardNavigation.nextStep crash

[jkmassel]

From Fabric:

Fatal Exception: NSInvalidArgumentException
Pushing the same view controller instance more than once is not supported (<WordPress.WebAddressWizardContent: 0x109031800>)

Fatal Exception: NSInvalidArgumentException
0  CoreFoundation                 0x187b01ea4 __exceptionPreprocess
1  libobjc.A.dylib                0x186cd1a50 objc_exception_throw
2  UIKitCore                      0x1b464a568 __71-[UINavigationController pushViewController:transition:forceImmediate:]_block_invoke
3  UIKitCore                      0x1b4649a28 -[UINavigationController pushViewController:animated:]
4  WordPress                      0x104926918 WizardNavigation.nextStep() (<compiler-generated>)
5  WordPress                      0x10492696c protocol witness for WizardDelegate.nextStep() in conformance WizardNavigation (<compiler-generated>)
6  WordPress                      0x1048af4e8 WebAddressStep.didSelect(_:) (WebAddressStep.swift:20)
7  WordPress                      0x1049f99e4 @objc WebAddressWizardContent.commitSelection() (<compiler-generated>)
8  UIKitCore                      0x1b4ca2314 -[UIApplication sendAction:to:from:forEvent:]
9  UIKitCore                      0x1b472fd54 -[UIControl sendAction:to:forEvent:]
10 UIKitCore                      0x1b4730074 -[UIControl _sendActionsForEvents:withEvent:]
11 UIKitCore                      0x1b472f074 -[UIControl touchesEnded:withEvent:]
12 UIKitCore                      0x1b4cdba6c -[UIWindow _sendTouchesForEvent:]
13 UIKitCore                      0x1b4cdccd0 -[UIWindow sendEvent:]
14 UIKitCore                      0x1b4cbbfcc -[UIApplication sendEvent:]
15 UIKitCore                      0x1b4d8ae38 __dispatchPreprocessedEventFromEventQueue
16 UIKitCore                      0x1b4d8d830 __handleEventQueueInternal
17 UIKitCore                      0x1b4d86320 __handleHIDEventFetcherDrain
18 CoreFoundation                 0x187a920e0 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
19 CoreFoundation                 0x187a92060 __CFRunLoopDoSource0
20 CoreFoundation                 0x187a91944 __CFRunLoopDoSources0
21 CoreFoundation                 0x187a8c810 __CFRunLoopRun
22 CoreFoundation                 0x187a8c0e0 CFRunLoopRunSpecific
23 GraphicsServices               0x189d05584 GSEventRunModal
24 UIKitCore                      0x1b4ca0c00 UIApplicationMain
25 WordPress                      0x10473fa80 main (main.m:11)
26 libdyld.dylib                  0x18754abb4 start

First Seen In: 11.9.0.2
Users Affected: 61 in the last 90 days
Source: 5c869d83f8b88c2963bc71ed-fabric

[designsimply]

Raising to [Pri] High because it appears to be a new crash introduced in 11.9.
_{(5c869d83f8b88c2963bc71ed-fabric-ios, p1554307992014100-slack-platform9)}

Noting that @ctarda and @stevebaranski were the iOS developers on the project and the next Groundkeeping rotation to match would be 6/24.
_{(p9wMUP-bk-p2)}

[ghost]

The stack trace suggests that we're pushing the same view controller onto the navigation stack more than once. That's frowned upon.

In this case, the user has selected _Create Site_, and we're attempting to service the site creation request. Perhaps the button was tapped multiple times?

To resolve this, two relatively straightforward approaches come to mind:

1. Disable UIButton selection after the first press here.
2. Double-check the contents of the navigation stack before pushing the view controller here.

I think one or both approaches could be applied.

@ctarda or @diegoreymendez - do you agree with my assessment? Would either of you have spare cycles to take a look? I am attending a meetup at the moment, so probably won't be able to tend to this until next week.

[diegoreymendez]

@stevebaranski - The message is very clear in the stack trace. I'd normally start checking exactly what you described.

I'll move this issue to the "Site Creation: iOS" project and try to check it out today.

[diegoreymendez]

I've tried reproducing this issue with multitaps in several ways, both in the simulator and in my iPhone XS without success. I realize it can be tricky, but I wonder if there's a chance there's some other issue.

Not being able to reproduce this problem makes me feel uneasy. I guess we could still make the change you proposed @stevebaranski and see how it goes in Fabric.

Thoughts?

[ghost]

Thanks for investigating this further @diegoreymendez.

Not being able to reproduce this problem makes me feel uneasy. I guess we could still make the change you proposed @stevebaranski and see how it goes in Fabric.

I see where you're coming from. We might be able to substantiate our claim by searching for duplicate
Tracks events for the same user.

In any case, if we decide not to proceed with a solution for v12.2, you are welcome to assign it back to me for further investigation.

[diegoreymendez]

Hey @stevebaranski, as I wasn't able to reach a satisfying conclusion for 12.2, I'm reassigning the issue to you. My apologies for that.

[diegoreymendez]

Removing this issue from the Site Creation project, as the project is wrapped up.

[ghost]

At present, I am unable to reproduce this when testing manually.

After reviewing stack traces in Fabric, I'm finding this issue isn't confined to a single step in the site creation flow. I found occurrences for each of the three intermediate steps in the process. That is to say:

1. VerticalsStep / VerticalsWizardContent (Step 1 of 3)
2. SiteInformationStep / SiteInformationWizardContent (2 of 3)
3. WebAddressStep / WebAddressWizardContent (3 of 3)

I have not found references to the initial step (i.e., Segments) or the final step (i.e., loading while the Site Creation request is executed).

This has led me to revise my initial assessment; I believe the problem lies somewhere in the implementation of WizardNavigation, where we calculate a "next" index with which to retrieve a view controller when advancing through the flow. This index can be modified in two ways:

1. When advancing forward, the index is incremented.
2. In a UINavigationControllerDelegate method; presumably this resets things when a user taps to navigate back in the flow.

I believe this index selection could be simplified.

[ghost]

v12.3.x has been out in the wild for a bit, and no crashes in Sentry have been found with this disposition.