Crash in java.lang.NullPointerException: Attempt to invoke virtual method ''int android.view.View.getWidth()'' on a null object reference at org.mozilla.gecko.menu.MenuPopup.showAsDropDown(MenuPopup.java)

RESOLVED FIXED in Firefox 54

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ahunt, Assigned: ahunt)

Tracking

({crash})

Trunk
Firefox 55
Unspecified
Android
crash
Points:
---

Firefox Tracking Flags

(firefox53 wontfix, firefox54+ fixed, firefox55+ fixed)

Details

(crash signature)

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-6a1a9d57-954c-48b6-b371-bb2c00170519.
=============================================================
(Assignee)

Comment 1

2 years ago
This looks like a regression from Bug 1282763:
https://hg.mozilla.org/mozilla-central/rev/073efe219a15#l1.34

If a user clicks on the menu button before the Runnable executes, the menu has no anchor, and we crash in MenuPopup.showAsDropDown().
(Assignee)

Comment 2

2 years ago
STR: convert postToUiThread() to postDelayedToUiThread() with a large delay (e.g. 2000). Build, run, open tabs tray, press menu button -> crashes.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8869558 - Flags: review?(cnevinchen)
Attachment #8869559 - Flags: review?(cnevinchen)
(Assignee)

Updated

2 years ago
status-firefox53: --- → affected
status-firefox54: --- → affected
status-firefox55: --- → affected
(Assignee)

Comment 7

2 years ago
It seems there are multiple instances in crash-stats, currently:
No. 54, 0.23%, 620 / 7 days
No. 70, 0.18%, 488 / 7 days

Not horrendous, but it might still be worthy of uplifting?
Crash Signature: [@ java.lang.NullPointerException: Attempt to invoke virtual method ''int android.view.View.getWidth()'' on a null object reference at org.mozilla.gecko.menu.MenuPopup.showAsDropDown(MenuPopup.java)] → [@ java.lang.NullPointerException: Attempt to invoke virtual method ''int android.view.View.getWidth()'' on a null object reference at org.mozilla.gecko.menu.MenuPopup.showAsDropDown(MenuPopup.java)] [@ java.lang.NullPointerException: at org.mozilla.geck…

Comment 8

2 years ago
mozreview-review
Comment on attachment 8869558 [details]
Bug 1366352 - Ensure anchor is always set before trying to show tabs tray menu

https://reviewboard.mozilla.org/r/141134/#review145756
Attachment #8869558 - Flags: review?(cnevinchen) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8869559 [details]
Bug 1366352 - Post: check anchor is set before trying to show GeckoPopupMenu

https://reviewboard.mozilla.org/r/141136/#review145760

::: mobile/android/base/java/org/mozilla/gecko/widget/GeckoPopupMenu.java:141
(Diff revision 2)
>      /**
>       * Show the inflated menu.
>       */
>      public void show() {
> +        if (mAnchor == null) {
> +            throw new IllegalStateException("GeckoPopupMenu.show() called without preceeding call to setAnchor()");

Why do we throw IllegalStateException here instead of just doing nothing?
Attachment #8869559 - Flags: review?(cnevinchen) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 12

2 years ago
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/66a8c78c4c24
Ensure anchor is always set before trying to show tabs tray menu r=nechen
https://hg.mozilla.org/integration/autoland/rev/80286a18d5d6
Post: check anchor is set before trying to show GeckoPopupMenu r=nechen

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/66a8c78c4c24
https://hg.mozilla.org/mozilla-central/rev/80286a18d5d6
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Please request Beta approval on this when you get a chance.
status-firefox53: affected → wontfix
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
Flags: needinfo?(andrzej)
Tracking 54/55+. Between the two signatures, I think it would be great to have this fixed in beta.
tracking-firefox54: ? → +
tracking-firefox55: ? → +
This has probably missed 54 now due to the lack of timely approval requests. Given the high volume we've seen on 53, there's probably good reason to suspect that it'll spike on 54 once that goes to release as well. Can you please help get this nominated for mozilla-release approval so it's at least on the radar for dot release consideration?
Flags: needinfo?(cnevinchen)
Comment on attachment 8869558 [details]
Bug 1366352 - Ensure anchor is always set before trying to show tabs tray menu

Approval Request Comment
[Feature/Bug causing the regression]: In tab tray, if Anchor is not set before menuPopup.showAsDropDown is called, we'll have a Null Pointer Exception 
[User impact if declined]:App will crash and it's hard to debug
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]:  No
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: It adds more check. and hook the UI base on System-based callback. So it should be safe.
[String changes made/needed]: No
Flags: needinfo?(cnevinchen)
Attachment #8869558 - Flags: approval-mozilla-release?
Comment on attachment 8869558 [details]
Bug 1366352 - Ensure anchor is always set before trying to show tabs tray menu

54 RC build is released today and it's late for 54. Release54-. Mark 54 won't fix.
Attachment #8869558 - Flags: approval-mozilla-release? → approval-mozilla-release-
status-firefox54: affected → wontfix
I'm setting this back to affected because I think this is high-volume enough on 53 to pay attention to after 54 goes to release...
status-firefox54: wontfix → affected
Flags: needinfo?(andrzej) → needinfo?(gchang)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #19)
> I'm setting this back to affected because I think this is high-volume enough
> on 53 to pay attention to after 54 goes to release...

Make sense. Keep tracking this in 54.
Flags: needinfo?(gchang)
Attachment #8869558 - Flags: approval-mozilla-release- → approval-mozilla-release?
Comment on attachment 8869558 [details]
Bug 1366352 - Ensure anchor is always set before trying to show tabs tray menu

Fix a crash. Release54+. Should be in 54.0.1.
Attachment #8869558 - Flags: approval-mozilla-release? → approval-mozilla-release+
Hello, the SV team looked over this and we could not reproduce the issue in our testing session.
You need to log in before you can comment on or make changes to this bug.