Leo Assignee
	Description • 11 years ago

1. Title: Crash occuring in MMS when killing the application while the MMS is sending
2. Precondition: Should be able to send and receive messages
3. Tester's Action:  (1) Open SMS application
                     (2) Select New Message 
                     (3) Send MMS with multiple images/Audio/Video
                     (4) Wait for a sometime while sending 
                     (5) kill the application
4. Detailed Symptom (ENG.) :The device crashes when killing the application and relaunching it again
5. Expected:  The device should not crash in any scenario
6. Reproducibility: Y
1) Frequency Rate : 100%

	Leo Assignee
	Comment 1 • 11 years ago

This issue was happening because even after the child process was killed we are trying to access the message contents (issue happens when accessing attachment).

We already have a check to see whether the content process is killed or not, but thats too late in this case.

To fix this issue we have to do the check little earlier.

	Leo Assignee
	Comment 2 • 11 years ago

Check whether the child process is killed or not.

	Chia-hung Tai [:ctai :ctai_mozilla :cht]
	Comment 3 • 11 years ago

Hi, Patrick,
How do think about this patch "attachment 799546 [details] [diff] [review]"?
Thanks.

	Andre Graziani (:graziani)
	Comment 4 • 11 years ago

Attaching the crash report, so it will be easier to understand the patch.

	Andre Graziani (:graziani)
	Comment 5 • 11 years ago

This is a second crash that I received related to this same issue.

But in this one, the crash happens after NotifyMessageGot (instead of NotifyMessageSent).

To leo.bugzilla.gaia: Shouldn't the same patch be applied to NotifyMessageGot also?

	Leo Assignee
	Comment 6 • 11 years ago

Thanks Andre,

We missed that one.
We will attach new patch after adding it.

	Leo Assignee
	Comment 7 • 11 years ago

Added more check based on Comment #5

	Patrick Wang (Chih-Kai Wang) (:kk1fff)
	Comment 8 • 11 years ago

Comment on attachment 799960 [details] [diff] [review]
Proposed patch

Review of attachment 799960 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/mobilemessage/src/ipc/SmsParent.cpp
@@ +25,5 @@
>  #include "nsTArrayHelpers.h"
>  
> +#undef LOG_TAG
> +#define LOG_TAG "MMSCPP"
> +#include <utils/Log.h>

Please remove the log here and __android_log_print below.

	Patrick Wang (Chih-Kai Wang) (:kk1fff)
	Comment 9 • 11 years ago

Comment on attachment 799960 [details] [diff] [review]
Proposed patch

Review of attachment 799960 [details] [diff] [review]:
-----------------------------------------------------------------

Please also check mActorDestroyed in other SmsRequestParent::Notify* functions. Thanks

	Vicamo Yang [:vicamo][:vyang]
	Comment 10 • 11 years ago

(In reply to Patrick Wang [:kk1fff] from comment #9)
> Please also check mActorDestroyed in other SmsRequestParent::Notify*
> functions. Thanks

They have already checks in |SendReply|.

	Vicamo Yang [:vicamo][:vyang]
	Comment 11 • 11 years ago

Comment on attachment 799960 [details] [diff] [review]
Proposed patch

Review of attachment 799960 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/mobilemessage/src/ipc/SmsParent.cpp
@@ +594,5 @@
>    nsCOMPtr<nsIDOMMozMmsMessage> mms = do_QueryInterface(aMessage);
> +
> +  __android_log_print(ANDROID_LOG_INFO, "GeckoMMS", "[Sharaf] SmsRequestParent::NotifyMessageSent %d", mActorDestroyed);
> +
> +  NS_ENSURE_TRUE(!mActorDestroyed, NS_ERROR_FAILURE);

Please move this check before |do_QueryInterface|.

  NS_ENSURE_TRUE(!mActorDestroyed, NS_ERROR_FAILURE);

  nsCOMPtr<nsIDOMMozMmsMessage> mms = do_QueryInterface(aMessage);
  if (mms) {
    ...

@@ +626,5 @@
>  SmsRequestParent::NotifyMessageGot(nsISupports *aMessage)
>  {
>    nsCOMPtr<nsIDOMMozMmsMessage> mms = do_QueryInterface(aMessage);
> +
> +  NS_ENSURE_TRUE(!mActorDestroyed, NS_ERROR_FAILURE);

ditto.

	Leo Assignee
	Comment 12 • 11 years ago

Hi,
Please find the updated patch with the review comments.
Thanks,

	Patrick Wang (Chih-Kai Wang) (:kk1fff)
	Comment 13 • 11 years ago

Comment on attachment 800611 [details] [diff] [review]
Patch for the issue

Review of attachment 800611 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.

	Vicamo Yang [:vicamo][:vyang]
	Comment 14 • 11 years ago

Comment on attachment 800611 [details] [diff] [review]
Patch for the issue

Sorry, just found this patch wasn't generated from hg and don't have information about the author.  We need email and user name of A human being to land this patch.

	Leo Assignee
	Comment 15 • 11 years ago

Hi,
Please find the updated patch with the format
Thanks,

	Vicamo Yang [:vicamo][:vyang]
	Comment 16 • 11 years ago

Comment on attachment 800693 [details] [diff] [review]
Patch with hg format

Review of attachment 800693 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you!

	Vicamo Yang [:vicamo][:vyang]
	Comment 17 • 11 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/8c7b45e8adc9

	Ed Morley [:emorley]
	Comment 18 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/8c7b45e8adc9

	Ryan VanderMeulen [:RyanVM]
	Comment 19 • 11 years ago

https://hg.mozilla.org/releases/mozilla-b2g18/rev/2ed8c6231e6b

	Ryan VanderMeulen [:RyanVM]
	Comment 20 • 11 years ago

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/2ed8c6231e6b