1

InternalStorageAllowedCheck is slow

 3 years ago
source link: https://bugzilla.mozilla.org/show_bug.cgi?id=1706292
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client
Closed Bug 1706292 Opened 5 months ago Closed 1 month ago

InternalStorageAllowedCheck is slow

Categories

(Core :: Privacy: Anti-Tracking, defect, P1)

Tracking

(bug RESOLVED as FIXED)

RESOLVED FIXED

92 Branch

Tracking Status firefox91 --- wontfix firefox92 --- fixed

People

(Reporter: smaug, Assigned: pbz)

References

(Blocks 2 open bugs)

Details

Moving component because there's no obvious connection to "DOM: Security" I'm seeing there, and it quite explicitly deals with LocalStorage. If I've missed something please add more explanation.

Component: DOM: Security → Storage: localStorage & sessionStorage

Searchfox says (in the "Navigation" side panel that people sometimes collapse) that the moz.build mappings for https://searchfox.org/mozilla-central/source/toolkit/components/antitracking/StorageAccess.cpp where the method is defined indicate this is Core :: Privacy : Anti-Tracking.

That said, nsGlobalWindowInner::GetLocalStorage could be further optimized. It likes to check all the time because the storage access permission can be granted and change what we should be returning, but it's possible for us to be event-driven; nsGlobalWindowInner::StorageAccessPermissionGranted exists and actually already has some LocalStorage specialization. It wouldn't be hard to optimize. However, arguably StoreAllowedForWindow/InternalStorageAllowedCheck could also be optimized to be fast given that every window explicitly holds an nsICookieJarSettings, etc.

Component: Storage: localStorage & sessionStorage → Privacy: Anti-Tracking
Severity: -- → S3
Priority: -- → P3

Looking at the code this could be helped a bit by the removal of lifetime policy checks, see bug 1681493.

See Also: → 1681493
Assignee: nobody → pbz
Status: NEW → ASSIGNED
Attachment #9233073 - Attachment description: WIP: Bug 1706292 - Do not clone principal for CookieJarSettings cookie permission check. r=timhuang! → Bug 1706292 - Do not clone principal for CookieJarSettings cookie permission check. r=timhuang!
Attachment #9233072 - Attachment description: WIP: Bug 1706292 - Cache InternalStorageAllowedCheck on inner window. r=timhuang! → Bug 1706292 - Cache InternalStorageAllowedCheck on inner window. r=timhuang!
Pushed by [email protected]:
https://hg.mozilla.org/integration/autoland/rev/4315ef065472
Do not clone principal for CookieJarSettings cookie permission check. r=timhuang,ckerschb

== Change summary for alert #30777 (as of Wed, 04 Aug 2021 11:05:44 GMT) ==

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new) 9% nytimes fnbpaint windows10-64-shippable-qr warm webrender 260.60 -> 237.38 8% nytimes fcp windows10-64-shippable-qr warm webrender 250.67 -> 231.88 7% nytimes fnbpaint windows10-64-shippable-qr warm webrender 259.94 -> 242.17

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30777

Comment hidden (obsolete)

Ok, that's odd, now I can reproduce locally. Investigating...

Flags: needinfo?(nbeleuzu) → needinfo?(pbz)

Comment on attachment 9233072 [details]
Bug 1706292 - Cache InternalStorageAllowedCheck on inner window. r=timhuang!

Revision D120833 was moved to bug 1724386. Setting attachment 9233072 [details] to obsolete.

Attachment #9233072 - Attachment is obsolete: true

I've moved the cache patch to Bug 1724386 since it won't land in time for 92 (soft freeze).

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
Flags: needinfo?(pbz)
You need to log in before you can comment on or make changes to this bug.

report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK