Bug 46692 - [Message Extension] Managed crash when calling base on MSMessagesAppViewController.DidStartSendingMessage or DidCancelSendingMessage
Summary: [Message Extension] Managed crash when calling base on MSMessagesAppViewContr...
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: General ()
Version: XI 10.2 (iOS 10.1)
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Vincent Dondain [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2016-11-10 01:28 UTC by Jon Goldberger [MSFT]
Modified: 2016-12-22 22:53 UTC (History)
3 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Test Project. (49.33 KB, application/zip)
2016-11-10 01:28 UTC, Jon Goldberger [MSFT]
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Jon Goldberger [MSFT] 2016-11-10 01:28:13 UTC
Created attachment 18401 [details]
Test Project.

## Description

Managed crash when calling base on MSMessagesAppViewController.DidStartSendingMessage or DidCancelSendingMessage. 

However the issue is because the message parameter _may_ be null, and if it is and you call base passing in the message parameter, then there is an exception message that the value cannot be null and the extension crashes. However when you use the override auto completion fro these two methods the base method is called in the method stub that is generated with no null checks on the message parameter. So I am unsure whether this is a bug on our end (the Apple docs [1][2] do not say whether message argument can be null or not) or perhaps it is that base should not be called at all on this method. 


[1] https://developer.apple.com/reference/messages/msmessagesappviewcontroller/1649186-didcancelsendingmessage
[2] https://developer.apple.com/reference/messages/msmessagesappviewcontroller/1649191-didstartsendingmessage

## Steps to reproduce

(I deployed to a device so I could view the device logs easily in XS, filtering for tag TestiMessageStickers.Ext)

1. Open the attached sample app. 

2. Deploy container app project to device or sim.

3. After app deploys, open Messages app.

4. Click on the "App" icon to open the Messages extension chooser and choose the TestiMessageStickers.Ext

5. Select the one of the stickers to send.

6. Cancel (hit the small "x" in the image about to be sent) or send the image. 

Expected result: The sticker browser will still be visible.

Actual result: "Unable to load TestiMessageStickers.Ext" is displayed where the sticker browser should be. 

## Notes

You can uncomment the try/catch blocks in for the call to base in those two methods to output the values of the two parameters to the console and output the managed exception message, which is "value cannot be null". 

## Workaround

Either don't override those methods unless needed, and if you do either do not call base or make sure message parameter is not null before calling base.

## Environment

=== Xamarin Studio Enterprise ===

Version 6.1.1 (build 17)
Installation UUID: ceaba76c-db06-4fbd-b326-f69ea53c3e01
Runtime:
	Mono 4.6.1 (mono-4.6.0-branch-c8sr0/ef43c15) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 406010005

=== NuGet ===

Version: 3.4.3.0

=== Xamarin.Profiler ===

Version: 0.33.1
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Xamarin.Android ===

Version: 7.0.1.3 (Visual Studio Enterprise)
Android SDK: /Users/jongoldberger/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		4.0.3 (API level 15)
		4.1   (API level 16)
		4.2   (API level 17)
		4.3   (API level 18)
		4.4   (API level 19)
		5.0   (API level 21)
		5.1   (API level 22)
		6.0   (API level 23)
		7.0   (API level 24)

SDK Tools Version: 25.2.2
SDK Platform Tools Version: 24.0.3
SDK Build Tools Version: 24.0.3

Java SDK: /usr
java version "1.8.0_91"
Java(TM) SE Runtime Environment (build 1.8.0_91-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.91-b14, mixed mode)

Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Xamarin Android Player ===

Version: 0.6.5
Location: /Applications/Xamarin Android Player.app

=== Xamarin Inspector ===

Version: 0.10.0.0
Hash: e931a52
Branch: master
Build date: Thu, 18 Aug 2016 17:46:46 GMT

=== Apple Developer Tools ===

Xcode 8.1 (11544)
Build 8B62

=== Xamarin.iOS ===

Version: 10.2.0.4 (Visual Studio Enterprise)
Hash: b638977
Branch: xcode8.1
Build date: 2016-10-25 14:38:48-0400

=== Xamarin.Mac ===

Version: 2.10.0.105 (Visual Studio Enterprise)

=== Build Information ===

Release ID: 601010017
Git revision: 44d481a9be9cf2bf7c01bfb99c59b77dfa08e712
Build date: 2016-10-25 14:27:24-04
Xamarin addins: 19cdb28081bf28d9688698030abb96e04e391f69
Build lane: monodevelop-lion-cycle8-sr0

=== Operating System ===

Mac OS X 10.12.1
Darwin Jons-MacBook-Pro.local 16.1.0 Darwin Kernel Version 16.1.0
    Thu Oct 13 21:26:57 PDT 2016
    root:xnu-3789.21.3~60/RELEASE_X86_64 x86_64

=== Enabled user installed addins ===

Xamarin Inspector 0.10.0.0
Comment 1 Sebastien Pouliot 2016-11-11 14:27:14 UTC
Apple headers indicate that null is not allowed, because there's no macro to allow it and because it's defined between

> NS_ASSUME_NONNULL_BEGIN

and 

> NS_ASSUME_NONNULL_END

macros. That might be a documentation/header mistake from Apple.

OTOH such an event (e.g. Start) makes no sense with a `null` value.

@Vincent can you have the first look since you did the bindings and, most likely, played a bit with the API ? thanks!
Comment 2 Vincent Dondain [MSFT] 2016-12-14 00:27:53 UTC
Hey,

So it seems the selector is happy if we pass it null.

However I have to say that Apple iMessage templates, unlike ours, do not call base by default, so maybe not the expected thing to do.

Side note: DidStartSendingMessage and DidCancelSendingMessage are not hit with Xcode 8.2 (XS and Xcode same behavior) and I don't know why.

@sebastien I'm not sure if we should update the template or add [NullAllowed] to the API.
Comment 3 Sebastien Pouliot 2016-12-22 15:14:36 UTC
@Vincent, let's not add [NullAllowed] that contradict Apple's headers. It's a event so the current code _probably_ does nothing (so null if fine) but that could change in the future.

I don't get the behaviour described, but that's likely because I'm using Xcode 8.2 - so it sounds like an issue that Apple fixed.

OTOH I think the template should be changed not to call the base methods (if Apple does not then we should not either).
Comment 4 Vincent Dondain [MSFT] 2016-12-22 22:53:03 UTC
iMessage extension template fixed in md-addins:master commit/60dafcee850c84f49c9a96423aedd960d2b4741f

https://github.com/xamarin/md-addins/commit/60dafcee850c84f49c9a96423aedd960d2b4741f

Since we're not gonna alter the bindings I'm considering this bug as resolved.