Don't import IndexedDBHelper.jsm and SettingsDB.jsm in preload.js

RESOLVED WONTFIX

Status

Firefox OS
Performance
P2
normal
RESOLVED WONTFIX
3 years ago
3 months ago

People

(Reporter: cyu, Assigned: cyu)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
IndexedDB is used through IPC in content processes. There's no need to load IndexedDBHelper.jsm and SettingsDB.jsm in preload.js.
(Assignee)

Comment 1

3 years ago
Oops! Accidentally marked as security-sensitive. Need help to make this bug public.
(Assignee)

Comment 2

3 years ago
Paul, would you help uncheck the security-sensitive flag? Thanks :)
Flags: needinfo?(ptheriault)
Group: b2g-core-security
Flags: needinfo?(ptheriault)
(Assignee)

Comment 4

3 years ago
Created attachment 8677950 [details] [diff] [review]
dont-preload-indexeddb-jsms.patch
Assignee: nobody → cyu
Attachment #8677950 - Flags: review?(fabrice)
SettingsDB.jsm is used by SettingsRequestManager.jsm which is then used by SettingsService.js
And this service can be used in child process unless I missed something, so we still benefit from preloading SettingsDB.jsm no?
(Assignee)

Comment 6

3 years ago
(In reply to [:fabrice] Fabrice Desré from comment #5)
> SettingsDB.jsm is used by SettingsRequestManager.jsm which is then used by
> SettingsService.js
> And this service can be used in child process unless I missed something, so
> we still benefit from preloading SettingsDB.jsm no?

SettingsRequestManager states that it only lives in the parent process:
https://dxr.mozilla.org/mozilla-central/source/dom/settings/SettingsRequestManager.jsm#1219
so I don't think i has any benefit in loading it in child processes.
Hm, yes - but we still load it when using the SettingsService in child processes. Maybe we should not do that instead?
(Assignee)

Comment 8

3 years ago
(In reply to [:fabrice] Fabrice Desré from comment #7)
> Hm, yes - but we still load it when using the SettingsService in child
> processes. Maybe we should not do that instead?

We shouldn't. I think we should also have a check in SettingsService that don't import SettingsRequestManager.jsm in the child process. The JSM is loaded only to throw an exception.
(Assignee)

Comment 9

3 years ago
After checking SettingsServiceLock, I think it only resides in the parent process because it has direct calls to SettingsRequestManager.

Updated

2 years ago
Priority: -- → P2
Attachment #8677950 - Flags: review?(fabrice)
(Assignee)

Updated

3 months ago
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.