SendTo Extension crashes when calling sendSyncPing

VERIFIED FIXED

Status

()

Firefox for iOS
General
P1
critical
VERIFIED FIXED
7 months ago
5 months ago

People

(Reporter: csuciu, Assigned: st3fan)

Tracking

({crash, regression, reproducible})

unspecified
Other
iOS
crash, regression, reproducible

Firefox Tracking Flags

(fxios8.0+, fxios-v7.0 unaffected, fxios-v8.0 affected)

Details

(Whiteboard: [MobileCore])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

7 months ago
Created attachment 8865393 [details]
SendTo  08-05-2017, 14-23.crash

master d43c368
iPhone 6 Plus (10.3.1)

1. Log into FxA
2. Open a webpage
3. Open Share menu
4. Select 'Send Tab'

Result: Firefox crashes
tracking-fxios: ? → 8.0+
Priority: -- → P1
Whiteboard: [MobileCore]
(Reporter)

Comment 1

7 months ago
The crash also happens when trying to 'Send Tab' from Safari
(Reporter)

Comment 2

7 months ago
Firefox Beta 8.0(5) is also affected.
Thread 3

Thread 3 Crashed:
0   libdispatch.dylib             	0x000000018971f800 dispatch_sync_f + 0
1   GCDWebServers                 	0x00000001027d2c38 GCDWebServerFormatRFC822 (GCDWebServerFunctions.m:120)
2   Telemetry                     	0x00000001016edac8 0x1016e4000 + 39624


Telemetry crash?

Updated

7 months ago
status-fxios-v7.0: --- → unaffected
Flags: needinfo?(sarentz)

Updated

6 months ago
Iteration: --- → 1.23
(Assignee)

Comment 4

6 months ago
I can't reproduce this bug on Fennec / master.
Flags: needinfo?(sarentz) → needinfo?(catalin.suciu)
(Reporter)

Comment 5

6 months ago
Created attachment 8875703 [details]
SendTo  08-06-2017, 15-19.crash

SendTo still crashes on master although not every time. Please see the new crash report, which is different than the one initially attached.

Also, Send Tab extension sometimes fails to display the list of connected devices (without crashing).

https://www.youtube.com/watch?v=n4bOU08H7ZI

Tested on master 4538e6d3e42 running on iPhone 6 Plus (10.3.2)
Flags: needinfo?(catalin.suciu)
(Reporter)

Comment 6

6 months ago
I can reproduce the above on Beta 8.0v3672 also.

Updated

6 months ago
Iteration: 1.23 → 1.24
P1 reproducible crash on 8.0 without an owner
Could you please test this again on the latest master. We had some changes to app groups
(Reporter)

Comment 9

6 months ago
I'm able to reproduce in latest master.

Send Tab is either crashing or doesn't display the list of connected devices.

Updated

6 months ago
Iteration: 1.24 → 1.25
(Assignee)

Updated

6 months ago
Assignee: nobody → sarentz
(Assignee)

Updated

5 months ago
Summary: [Regression] Crash in SendTo PlugInKit: [PKService run] + 752 → SendTo Extension crashes when calling sendSyncPing
(Assignee)

Comment 10

5 months ago
Created attachment 8883620 [details] [review]
PR https://github.com/mozilla-mobile/firefox-ios/pull/2885

We should not be calling `sendSyncPing` when running outside of the application. This patch checks if `profile.app` is set to find out if we are in the app or an extension. This is not ideal but there is no other good way to discover this at runtime it seems.
Attachment #8883620 - Flags: review?(jhugman)
(Assignee)

Updated

5 months ago
Status: NEW → ASSIGNED
(Assignee)

Updated

5 months ago
Whiteboard: [MobileCore] → [MobileCore][needsuplift]
(Assignee)

Updated

5 months ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
Whiteboard: [MobileCore][needsuplift] → [MobileCore]
I don't think this patch fixes the problem.

profile.app is no longer used. The property should've been deleted as part of Bug 1373202. We should also be using a method to hide the implementation details of isApplicationProfile().
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8883620 - Flags: review?(jhugman) → feedback+
(Assignee)

Comment 12

5 months ago
Created attachment 8883794 [details] [review]
PR https://github.com/mozilla-mobile/firefox-ios/pull/2892

This patch introduces an `AppInfo.isApplication` computed property that will tell us if we are running as part of an UIApplication. This is used to decide if we need to collect sync ping info.
Attachment #8883620 - Attachment is obsolete: true
Attachment #8883794 - Flags: review?(jhugman)
Attachment #8883794 - Flags: review?(jhugman) → review+
Excellent!
(Assignee)

Comment 14

5 months ago
New PR landed and uplifted to v8.x
Status: REOPENED → RESOLVED
Last Resolved: 5 months ago5 months ago
Resolution: --- → FIXED
(Reporter)

Comment 15

5 months ago
I can't reproduce the crash in Beta 8.0 (4467)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.