BouncerService overrides a method getDataDir introduced in API24 (N)

NEW
Unassigned

Status

Android Background Services
Geolocation
a year ago
5 months ago

People

(Reporter: Grisha, Unassigned, Mentored, NeedInfo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(3 attachments)

(Reporter)

Description

a year ago
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
(Reporter)

Updated

a year ago
Whiteboard: [good first bug]
(Reporter)

Updated

a year ago
Mentor: gkruglov@mozilla.com

Comment 1

a year ago
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

a year ago
Created attachment 8816894 [details] [diff] [review]
Bug-1292367-BouncerService overrides a method getDataDir introduced in API24 (N).patch
Attachment #8816894 - Flags: feedback?(gkruglov)

Comment 3

a year ago
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.
Flags: needinfo?(gkruglov)
(Reporter)

Comment 4

8 months ago
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
Flags: needinfo?(gkruglov) → needinfo?(thomascharles94)
(Reporter)

Comment 5

8 months ago
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.
Attachment #8816894 - Flags: feedback?(gkruglov) → feedback+

Comment 6

8 months ago
Hi due to current time commitments I will not be able to continue work on this bug
Flags: needinfo?(thomascharles94)

Comment 7

6 months ago
Created attachment 8876010 [details] [diff] [review]
Rename getDataDir to getDataDirString in BouncerService

Updated

6 months ago
Attachment #8876010 - Flags: review?

Updated

6 months ago
Attachment #8876010 - Flags: review? → review?(gkruglov)

Comment 8

5 months ago
(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

5 months ago
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

5 months ago
Created attachment 8881369 [details] [diff] [review]
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
Flags: needinfo?(gkruglov)
You need to log in before you can comment on or make changes to this bug.