599996 - Permission manager's nsPermissionManager::Import should be removed

Status: RESOLVED FIXED

(Core :: Permission Manager, defect)

Description

[Shawn Wilsher :sdwilsh]

Maybe reading too, but we should stop writing to disk on the main thread at the very least.

Comment 1

[:Gijs (he/him)]

Doug, is there anything to do here besides bug 975996? If not, perhaps we should just dupe it to that bug given it's got an assignee and patches and so on?

Comment 2

[Alex Thayer [:alexical] (she/her)]

Eh - the main thread IO in nsPermissionManager::Import() is at least nominally outside the scope of bug 975996, since it's IO on a different file, but that's the only thing I can see. So we could either rename this or rename bug 975996 to account for that.

Comment 3

[:Gijs (he/him)]

Huh. Looks to me like we never have a 'hostperms.1' file anymore, except in reeeeeaaally old profiles. We switched away from it at least 4 years ago, some of it happened 11 years ago. If you've run any Firefox release in the last 4 years the file would have been removed anyway. We should just remove the import code. p2 because this is actually still going to cause main-thread IO on every startup because we always try to read this file (and then realize it doesn't exist, for pretty well all profiles) - even if it isn't much, that's 1 more cause of main-thread IO that we can pretty easily get rid of.

Comment 4

[:Gijs (he/him)]

MattN flagged up that we should check with the Android folks given bug 1072744. I don't see any hits for Android either, so I think we might still be OK, but we should check.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

It looks like hostperms.1 may have been the old flat file for storing permissions back before Firefox 3..

I suspect this is mostly leftover migration code. Hey snorp, when we run Gecko for the first time on Android, do you know if we rely on creating a hostperms.1 file or anything to pre-populate the permissions database?

If not, I think this is ripe for removal.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Hey Emma,

If you're interested, this looks like a nice opportunity to get rid of some ancient main-thread IO in the start-up path. If you decide to take this, maybe check with snorp or liuche from the mobile team to make sure they're not somehow still using hostperms.1, and then we can probably drop this import check.

Comment 7

[Emma Malysz]

Attachment: Bug 599996, remove nsPermissionManager::Import to avoid main-thread IO

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f4f0c7453b9
remove nsPermissionManager::Import to avoid main-thread IO r=mconley

Comment 9

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

Yeah, we certainly don't need this for GeckoView, so it's fine to remove.

Comment 10

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/9f4f0c7453b9