1292367 - BouncerService overrides a method getDataDir introduced in API24 (N)

Status: RESOLVED INVALID

(Android Background Services Graveyard :: Geolocation, defect)

Description

[:Grisha Kruglov]

From the linter report:

Starting with API 24, ContextWrapper has a getDataDir(): https://developer.android.com/reference/android/content/ContextWrapper.html#getDataDir()

BouncerService also defines a getDataDir method, and will conflict with new inherited method once we move build target to 24+. https://dxr.mozilla.org/mozilla-central/source/mobile/android/bouncer/java/org/mozilla/bouncer/BouncerService.java#98

Comment 1

[Raphael Cojocaru]

Hi! I would like to work on this bug. 
As you said before, the bug is that BouncerService.java has a method called "getDataDir()", the same name used by ContextWrapper. So this bug can be simply solved by renaming the method within BouncerService.java?

Comment 2

[Thomas Charles]

Attachment: Bug-1292367-BouncerService overrides a method getDataDir introduced in API24 (N).patch

Comment 3

[Thomas Charles]

Hi,

I noticed there had been no activity on this for a while so I have had a go at resolving this bug.

I have attached a patch with a potential fix.

If this approach is correct can I have this bug assigned to me?

Thanks.

Comment 4

[:Grisha Kruglov]

Apologies for a _very_ late reply! Let me know if you're still interested in taking this bug.

We might need this for API26 support, e.g. https://reviewboard.mozilla.org/r/125648/diff/1#index_header

Comment 5

[:Grisha Kruglov]

Comment on attachment 8816894 [details] [diff] [review]
Bug-1292367-BouncerService overrides a method getDataDir introduced in API24 (N).patch

Since this is a private method that I think is just used once, I'd vote for "delete this method" entirely. Otherwise, renaming should be fine as well.

Comment 6

[Thomas Charles]

Hi due to current time commitments I will not be able to continue work on this bug

Comment 7

[Kai Lin]

Attachment: Rename getDataDir to getDataDirString in BouncerService

Comment 8

[shubham kumar singh]

(In reply to Kai Lin from comment #7)
> Created attachment 8876010 [details] [diff] [review]
> Rename getDataDir to getDataDirString in BouncerService

How many changes do we have to make if we change this method name here. If this method is used at other places in the codebase. Can't we just keep it getDataDir() and override the method in the ContextWrapper, plus add a method for calling the ContextWrapper method like this:

  @Override
  private String getDataDir() {
        return getApplicationInfo().dataDir;
    }
 private File sGetDataDir() {
 	return super.getDataDir();
 }



I'm just assuming that we are going to use the overridden method more than the predefined one. Kindly correct me if I'm wrong

Comment 9

[shubham kumar singh]

Sorry but that wouldn't work but I guess this should. Can't be sure because I cant test it. Can anyone help me out with it

Here's the changes that I have made
 @Override
    private String getDataDir() {
        return getApplicationInfo().dataDir;
    }

    ContextWrapper cw = ContextWrapper(Context base);
    private String aGetDataDir() {
    	return cw.getDataDir();
    }

Need help in testing this and creating a patch.....

Comment 10

[shubham kumar singh]

Attachment: It contains an additional method for calling the Android getDataDir() method leaving the method in BoouncerService.java unchanged

This is the diff file about containing the new aGetDataDir() method which leaves the getDataDir() method in BouncerService.java unchanged. I haven't tested and I need help in this regard

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

BouncerService seems to be dead: https://searchfox.org/mozilla-central/search?q=path%3Amobile%2Fandroid+bouncer&path=