Closed Bug 1488434 Opened Last year Closed Last year

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

Categories

(Core :: Web Speech, defect, critical)

Unspecified
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- wontfix
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: marcia, Assigned: eeejay)

References

()

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file, 1 obsolete file)

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@framework@boot.art system@framework@boot.art@0x17649a 
8 libart.so libart.so@0x40d575 
9 dalvik-main space (region space) (deleted) dalvik-main space @0xac581e 

=============================================================
Adding 62 as affected since there are a handful of crashes. Still pretty low volume.
Andre, can you take a look at this? Obrigado!
Flags: needinfo?(anatal)
I'm more familiar with the recognition portion of the Web Speech API. For synthesis, it is better to ask eejay.
Flags: needinfo?(anatal) → needinfo?(eitan)
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..
Attachment #9013834 - Flags: review?(nchen)
Assignee: nobody → eitan
Flags: needinfo?(eitan)
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
Attachment #9013834 - Flags: review?(nchen) → review+
Attachment #9013834 - Attachment is obsolete: true
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8b45fb6a89d
Check sTTS is not null before using it. r=jchen
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c8b45fb6a89d
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Please request Beta approval on this when you get a chance.
Flags: needinfo?(eitan)
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
Flags: needinfo?(eitan)
Attachment #9014103 - Flags: approval-mozilla-beta?
Comment on attachment 9014103 [details] [diff] [review]
Check sTTS is not null before using it.

Crash fix, uplift approved for 63 beta 13, thanks.
Attachment #9014103 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.