1241190 - Crash in UserActivity: +[UAUserActivity(Internal) checkWebpageURL:actionType:throwIfFailed:] + 320

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Aaron Train [:aaronmt]]

Last Exception Backtrace:
0   CoreFoundation                	0x180e45518 __exceptionPreprocess + 124 (NSException.m:162)
1   libobjc.A.dylib               	0x1804abf80 objc_exception_throw + 56 (objc-exception.mm:531)
2   UserActivity                  	0x190c92e58 +[UAUserActivity(Internal) checkWebpageURL:actionType:throwIfFailed:] + 320 (UAUserActivity.m:2285)
3   UserActivity                  	0x190c8c9ac -[UAUserActivity setWebpageURL:] + 84 (UAUserActivity.m:570)
4   Client                        	0x10013879c 0x10004c000 + 968604
5   Client                        	0x100138e10 0x10004c000 + 970256
6   Client                        	0x100137f9c 0x10004c000 + 966556
7   Client                        	0x100138094 0x10004c000 + 966804
8   Client                        	0x1001e097c 0x10004c000 + 1657212
9   Client                        	0x1001de104 0x10004c000 + 1646852

Comment 1

[Richard Newman [:rnewman]]

Needs symbols.

Comment 2

[Stephan Leroux [:sleroux]]

Symbolized Client parts:

specialized SpotlightHelper.update([String : String], forURL : NSURL) -> () (in Client) (SpotlightHelper.swift:0)
specialized SpotlightHelper.userContentController(WKUserContentController, didReceiveScriptMessage : WKScriptMessage) -> () (in Client) (SpotlightHelper.swift:124)
@objc SpotlightHelper.userContentController(WKUserContentController, didReceiveScriptMessage : WKScriptMessage) -> () (in Client) (SpotlightHelper.swift:0)
protocol witness for BrowserHelper.userContentController<A where ...>(WKUserContentController, didReceiveScriptMessage : WKScriptMessage) -> () in conformance SpotlightHelper (in Client) (SpotlightHelper.swift:120)
specialized (HelperManager in _51AAA34A1E0B8817D1AAA32D321A88D7).userContentController(WKUserContentController, didReceiveScriptMessage : WKScriptMessage) -> () (in Client) (Browser.swift:414)
@objc (HelperManager in _51AAA34A1E0B8817D1AAA32D321A88D7).userContentController(WKUserContentController, didReceiveScriptMessage : WKScriptMessage) -> () (in Client) (Browser.swift:0)

Comment 3

[Aaron Train [:aaronmt]]

This is on the 2.0 signature listing in XCode.

Comment 4

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

We need some movement on this bug. It is the top crasher for our most recent two 2.0 builds. 

I a very worried about this because this code is running every single time a page finishes loading. So the changes that this becomes a real production top crasher are extremely high.

If we can't figure out what is happening here then we have to think about delaying shipping Spotlight until it is more stable.

Comment 5

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

I can't see the crashes in Organizer right now. Is this 8.0 only, 9.0 only?

The code flow seems to be (if I'm reading the log right, and then referencing the code): 

userActivity = NSUserActivity(String)
userActivity.webpageURL = url

Comment 6

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

I also thought that this might be to do with how well or poorly we're validating what is coming out of the JS. 

The JS guarantees to produce a dictionary of [String: String]. The handler gets called on each page load, with a dictionary with keys "title" and "description". If the values don't exist, from the HTML, the JS defaults it to an empty string.

Comment 7

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

Discussion with :sleroux and :st3fan

Two theories:
 a) something strange is coming back from JS, but swift isn't handling it well.
 b) NSURL can contain some badly formed URLs that don't pass muster for the NSUserActivity.

(a) has form in the LoginsHelper, https://github.com/mozilla/firefox-ios/pull/1140 but the null case is specifically handled in the JS so it passes back an empty string, rather than NSNull or nil.

(b) sounds plausible. The webpageURL isn't actually needed for the functioning of this feature. Using something else just means that users who uninstall Firefox won't fall back to using Safari, and the results go simply away after uninstall (I think).

Comment 8

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

Attachment: Pull request

Comment 9

[Stephan Leroux [:sleroux]]

Comment on attachment 8713688 [details] [review]
Pull request

Looks good. I tried running on device and searching spotlight for 'about' but the about tab but it wasn't showing up so I wasn't able to test this through spotlight.

Comment 10

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

Merged.