Closed Bug 1316005 Opened 8 years ago Closed 7 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect, P3)

All
Android
defect

Tracking

(firefox53 fixed)

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: sebastian, Assigned: svezauzeto12, Mentored)

References

Details

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

Attachments

(1 file)

      No description provided.
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
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.
I just answered the same question in bug 1316023 - Let's start with this one. :)
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
I will take this one
I will take this one.
Assignee: nobody → svezauzeto12
Status: NEW → ASSIGNED
I have made patch for the bug, what is the procedure now ?

Thanks.
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)
(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).
I will try to do what is written above and then commit.
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+
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
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 )
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6459d80bf245d1f5e2f46bda884878548d58126
Bug 1316005 - fixed unused methods, made methods use actual signature and added @Override;  r=sebastian
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. :)
https://hg.mozilla.org/mozilla-central/rev/e6459d80bf24
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: