1401282 - Sending a null intent to the search activity crashes Firefox

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Search Activity, defect)

Description

[Mike Kaply [:mkaply]]

adb shell am broadcast -n org.mozilla.firefox/org.mozilla.search.SearchWidget

crahes Firefox.

Comment 1

[Mike Kaply [:mkaply]]

https://crash-stats.mozilla.com/report/index/c73e00bf-133f-474c-9c82-f08900170929

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Bug 1401282 - Don't assume the intent has an action.

Review commit: https://reviewboard.mozilla.org/r/185262/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185262/

Comment 3

[Mike Kaply [:mkaply]]

[Tracking Requested - why for this release]: Crash that partner found while working on preload.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8913865 [details]
Bug 1401282 - Don't assume the intent has an action.

https://reviewboard.mozilla.org/r/185262/#review190644

::: mobile/android/search/java/org/mozilla/search/SearchWidget.java:68
(Diff revision 1)
>      @Override
>      public void onReceive(final Context context, final Intent intent) {
>          // This will hold the intent to redispatch.
> -        final Intent redirect;
> -        switch (intent.getAction()) {
> +        Intent redirect = null;
> +        final String action = intent.getAction();
> +        if (action != null) {

Best to just rearly-return here to avoid having to indent the entire switch block.


if (action == null) {
    super.onReceive(context, intent);
    return;
}

Comment 5

[Mike Kaply [:mkaply]]

Comment on attachment 8913865 [details]
Bug 1401282 - Don't assume the intent has an action.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/185262/diff/1-2/

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8913865 [details]
Bug 1401282 - Don't assume the intent has an action.

https://reviewboard.mozilla.org/r/185262/#review190680

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8913865 [details]
Bug 1401282 - Don't assume the intent has an action.

https://reviewboard.mozilla.org/r/185262/#review190684

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d300dade1d49
Don't assume the intent has an action. r=snorp

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 10

[Mike Kaply [:mkaply]]

Comment on attachment 8913865 [details]
Bug 1401282 - Don't assume the intent has an action.

Approval Request Comment
[Feature/Bug causing the regression]: Simple crasher
[User impact if declined]: Crash with null intent
[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]: In bug
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just bailing out early if there is no action.
[String changes made/needed]: None

I know this seems an odd one to request uplift, but a partner found it (working on a new phone) and it's a simple crasher fix.

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8913865 [details]
Bug 1401282 - Don't assume the intent has an action.

Partner ask, not-null check, Beta57+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f31efa9dfa8f

Comment 13

[Bogdan Surd, QA [:bsurd, NI]]

Verified this using the command line listed in Comment 0 on a Huawei Media Pad 2 (5.1.1). Fennec did no longer crash.