1488434 - Crash in java.lang.NullPointerException: at org.mozilla.gecko.SpeechSynthesisService.setUtteranceListener(SpeechSynthesisService.java)

Status: RESOLVED FIXED

(Core :: Web Speech, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-b5417868-14e4-4b10-8595-859710180903.
=============================================================

Seen while running on my Pixel 2. Before the crash I was getting the microphone dialog and had to dismiss several times. At the time I was visiting https://www.accuweather.com/en/weather-news/florida-us-gulf-coast-on-alert-for-budding-tropical-depression/70005956.

Top 10 frames of crashing thread:

0 libxul.so GeckoAppShellSupport::ReportJavaCrash widget/android/nsAppShell.cpp:281
1 libxul.so void mozilla::jni::NativeStub<mozilla::java::GeckoAppShell::ReportJavaCrash_t, GeckoAppShellSupport, mozilla::jni::Args<mozilla::jni::Ref<mozilla::jni::TypedObject<_jthrowable*>, _jthrowable*> const&, mozilla::jni::StringParam const&> >::Wrap<&GeckoAppShellSupport::ReportJavaCrash> widget/android/jni/Natives.h:778
2 base.odex base.odex@0x225b3 
3 dalvik-LinearAlloc (deleted) dalvik-LinearAlloc @0x49b2 
4 dalvik-main space (region space) (deleted) dalvik-main space @0x631cde 
5 dalvik-main space (region space) (deleted) dalvik-main space @0xac581e 
6 dalvik-main space (region space) (deleted) dalvik-main space @0xad7c96 
7 system@[e-mail] system@[e-mail]@0x17649a 
8 libart.so libart.so@0x40d575 
9 dalvik-main space (region space) (deleted) dalvik-main space @0xac581e 

=============================================================

Comment 1

[Marcia Knous [:marcia]]

Adding 62 as affected since there are a handful of crashes. Still pretty low volume.

Comment 2

[Mike Taylor [:miketaylr]]

Andre, can you take a look at this? Obrigado!

Comment 3

[André Natal]

I'm more familiar with the recognition portion of the Web Speech API. For synthesis, it is better to ask eejay.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Check sTTS is not null before using it. r?jchen

I'm puzzled by how sTTS could be null there. Maybe if the Init listener is called before the sTTS constructor returns? Anyway, I added null checks everywhere for good measure..

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9013834 [details] [diff] [review]
Check sTTS is not null before using it. r?jchen

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

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/SpeechSynthesisService.java
@@ +88,5 @@
>      private static native void doneRegisteringVoices();
>  
>      @WrapForJNI(calledFrom = "gecko")
>      public static String speak(final String uri, final String text, float rate, float pitch, float volume) {
> +        if (sTTS == null) { return null; }

Use multiple lines and log a warning first (e.g. `Log.w(LOGTAG, "...");`)

@@ +105,5 @@
>          return utteranceId;
>      }
>  
>      private static void setUtteranceListener() {
> +        if (sTTS == null) { return; }

Same here

@@ +152,5 @@
>      private static native void dispatchBoundary(String utteranceId, int start, int end);
>  
>      @WrapForJNI(calledFrom = "gecko")
>      public static void stop() {
> +        if (sTTS == null) { return; }

Same here

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Check sTTS is not null before using it.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8b45fb6a89d
Check sTTS is not null before using it. r=jchen

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c8b45fb6a89d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 10

[Eitan Isaacson [:eeejay]]

Comment on attachment 9014103 [details] [diff] [review]
Check sTTS is not null before using it.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1184142

User impact if declined: Occasional crashes

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This is just a few null checks.

String changes made/needed: no

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9014103 [details] [diff] [review]
Check sTTS is not null before using it.

Crash fix, uplift approved for 63 beta 13, thanks.

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0978af5fb845