[findbugs] [UMAC] Uncallable method defined in anonymous class

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
General
P3
normal
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: sebastian, Assigned: Tomislav Jurin, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 53
All
Android
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [lang=java][good first bug])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Comment 1

6 months ago
Uncallable method org.mozilla.gecko.MediaPlayerManager$1.onRouteSelected(MediaRouter, int, MediaRouter$RouteInfo) defined in anonymous class

Uncallable method org.mozilla.gecko.MediaPlayerManager$1.onRouteUnselected(MediaRouter, int, MediaRouter$RouteInfo) defined in anonymous class

"This anonymous class defined a method that is not directly invoked and does not override a method in a superclass. Since methods in other classes cannot directly invoke methods declared in an anonymous class, it seems that this method is uncallable. The method might simply be dead code, but it is also possible that the method is intended to override a method declared in a superclass, and due to an typo or other error the method does not, in fact, override the method it is intended to."
Priority: -- → P3
Summary: [findbugs] [UMAC] Uncallable method org.mozilla.gecko.MediaPlayerManager$1.onRouteSelected(MediaRouter, int, MediaRouter$RouteInfo) defined in anonymous class → [findbugs] [UMAC] Uncallable method defined in anonymous class

Comment 2

6 months ago
Hi Sebastian!
I would like to work on this bug. Can you explain me in brief what i need to do given that this is my first bug.
(Reporter)

Comment 3

6 months ago
I just answered the same question in bug 1316023 - Let's start with this one. :)
(Reporter)

Comment 4

6 months ago
Anyways, for whoever picks this one up, ..

.. to start, set up a build environment - you can see the instructions here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

.. then, you'll need to upload a patch - see:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "sebastian" and you can find me and other helpful folks in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC
(Assignee)

Comment 5

5 months ago
I will take this one
(Assignee)

Comment 6

5 months ago
I will take this one.
Comment hidden (mozreview-request)
(Reporter)

Updated

5 months ago
Assignee: nobody → svezauzeto12
Status: NEW → ASSIGNED
(Assignee)

Comment 8

5 months ago
I have made patch for the bug, what is the procedure now ?

Thanks.
(Reporter)

Comment 9

5 months ago
mozreview-review
Comment on attachment 8812494 [details]
Bug 1316005 - fixed unused methods, made methods use actual signature and added @Override;removed additional line

https://reviewboard.mozilla.org/r/94220/#review94566

::: mobile/android/base/java/org/mozilla/gecko/MediaPlayerManager.java
(Diff revision 1)
> -            @SuppressWarnings("unused")
> -            public void onRouteSelected(MediaRouter router, int type, MediaRouter.RouteInfo route) {
> -                updatePresentation();
> -            }
> -
> -            // These methods aren't used by the support version Media Router
> -            @SuppressWarnings("unused")
> -            public void onRouteUnselected(MediaRouter router, int type, RouteInfo route) {
> -                updatePresentation();
> -            }

Looking at the current version of MediaRouter.Callback I think it would make sense to move to the actual signature (without 'int type'):

https://developer.android.com/reference/android/support/v7/media/MediaRouter.Callback.html#onRouteSelected(android.support.v7.media.MediaRouter,%20android.support.v7.media.MediaRouter.RouteInfo)

After that we can remove the SuppressWarnings annotation and add the Override annotation.
Attachment #8812494 - Flags: review?(s.kaspari)
(Reporter)

Comment 10

5 months ago
(In reply to svezauzeto12 from comment #8)
> I have made patch for the bug, what is the procedure now ?

I just reviewed the current version of the patch. After it has passed review I'll push the patch to the test servers and if it passes all tests then we can land the patch (by adding the 'checkin-needed' keyword to the bug).
(Assignee)

Comment 11

5 months ago
I will try to do what is written above and then commit.
Comment hidden (mozreview-request)
(Reporter)

Comment 13

5 months ago
mozreview-review
Comment on attachment 8812494 [details]
Bug 1316005 - fixed unused methods, made methods use actual signature and added @Override;removed additional line

https://reviewboard.mozilla.org/r/94220/#review95490

LGTM.

::: mobile/android/base/java/org/mozilla/gecko/MediaPlayerManager.java:199
(Diff revision 2)
> -            @SuppressWarnings("unused")
> -            public void onRouteSelected(MediaRouter router, int type, MediaRouter.RouteInfo route) {
> +            @Override
> +            public void onRouteSelected(MediaRouter router, MediaRouter.RouteInfo route) {
>                  updatePresentation();
>              }
>  
> -            // These methods aren't used by the support version Media Router
> +

nit: Remove this additional empty line.
Attachment #8812494 - Flags: review?(s.kaspari) → review+
(Reporter)

Comment 14

5 months ago
I pushed the patch to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=27a11491fe80
(Assignee)

Comment 15

5 months ago
To remove this line I should just make new commit right ? Or should I make new commit and merge it with the previous one for this issue ? 

Thanks for information
(Assignee)

Comment 16

5 months ago
I have removed additional line and squashed it with last commit, so you should have all changes in this commit now.

When you are happy with how this is done I would appreciate if you could suggest new bug for me to work on ( I would prefer something that maybe has some actually coding, but if it is too early for that I will accept anything )
Comment hidden (mozreview-request)
(Reporter)

Comment 18

5 months ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6459d80bf245d1f5e2f46bda884878548d58126
Bug 1316005 - fixed unused methods, made methods use actual signature and added @Override;  r=sebastian
(Reporter)

Comment 19

5 months ago
Thank you for the patch!

(In reply to Tomislav Jurin from comment #16)
> When you are happy with how this is done I would appreciate if you could
> suggest new bug for me to work on ( I would prefer something that maybe has
> some actually coding, but if it is too early for that I will accept anything

Good question. We are running a bit low on mentor bugs currently. But looking through the bugs I filed recently. Maybe bug 1312686? It's still simple but you can actually change something that is visible to the user. :)

Comment 20

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e6459d80bf24
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.