Closed Bug 1669437 Opened 4 years ago Closed 4 years ago

LocalStorage can’t be shared across tabs or subdomains when using private browsing options

Categories

(Core :: Storage: localStorage & sessionStorage, defect)

Product:
Core
Component:
Storage: localStorage & sessionStorage
Version:
Firefox 81
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
84 Branch
Tracking Flags:
Tracking Status
firefox84 --- verified

People

(Reporter: rob, Assigned: janv)

References

Details

Attachments

(4 files, 3 obsolete files)

Bug 1669437 - Get rid of the private data set; r=#dom-workers-and-storage-reviewers,asuth
47 bytes, text/x-phabricator-request
Details | Review
Bug 1669437 - Add necessary infrastructure for independent in-memory only local storage database; r=#dom-workers-and-storage-reviewers,asuth
47 bytes, text/x-phabricator-request
Details | Review
Bug 1669437 - Enable full sharing of private browsing data across content processes (using the in-memory database in the parent process); r=#dom-workers-and-storage-reviewers,asuth
47 bytes, text/x-phabricator-request
Details | Review
A patch for local testing (it forces creation of new content processes for each tab)
1.29 KB, patch
Details | Diff | Splinter Review

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0 Safari/605.1.15

Steps to reproduce:

  1. Any of:
  • Open a new private window in Firefox
  • Check "Delete cookies and site data when Firefox is closed" in preferences -> privacy & security -> cookies and site data of Firefox
  • Use "permanent private browsing mode", as in the Tor browser
  1. Sign into the latest ProtonMail beta (beta.protonmail.com)
  2. Open ProtonCalendar from the app dropdown in the top left of the navigation bar

Actual results:

  1. A new tab is opened to calendar.protonmail.com
  2. The new tab directs to account.protonmail.com and looks for a session stored in LocalStorage
  3. No session is found (despite the first tab having set one) and so the tab requests the user to sign in anew

Expected results:

  1. [Same as above] A new tab is opened to calendar.protonmail.com
  2. [Same as above] The new tab directs to account.protonmail.com and looks for a session stored in LocalStorage
  3. A session is found in LocalStorage (set by the first tab)
  4. New tab directs back to calendar.protonmail.com with the session information

This bug might be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1453699, as following one of the suggestions to enable dom.storage.next_gen in beta/nightly indeed resolves the issue. Can someone please provide an update of this new storage solution coming to production?

Assignee: nobody → jvarga
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Adding qe-verify to ensure it's on QA's radar for validating that ProtonMail and Office365 issues are fixed by this bug.

Flags: qe-verify?

The caches for private browsing are already separated by the private browsing
origin attribute, so there's no need to have a special data set (inside the
cache object) for it.

Depends on D96527

This patch creates a separate bridge between the content and the parent process
and also a separate local storage database in the parent process for storing and
sharing private browsing data across content processes.
The synchronization of private browsing data to the parent process is not yet
enabled.

Depends on D96531

The missing % resulted in returning zero usage.

Depends on D96566

This patch handles the case when GetBaseDomain returns an empty string in
BasePrincipal::GetLocalStorageQuotaKey. The method would return a very generic
scope otherwise, causing the usage calculation to return usage for all origins. LocalStorageCache::Init falls back to the origin scope for usage calculation
when the domain scope returned by BasePrincipal::GetLocalStorageQuotaKey is
empty or the method fails.

Depends on D96578

Comment on attachment 9186909 [details]
Bug 1669437 - Fix a regression caused by a change done in bug 1165214; r=#dom-workers-and-storage-reviewers,asuth

Revision D96578 was moved to bug 1676410. Setting attachment 9186909 [details] to obsolete.

Attachment #9186909 - Attachment is obsolete: true

Comment on attachment 9186914 [details]
Bug 1669437 - Fix usage calculation for file:// (and potentially other) URIs; r=#dom-workers-and-storage-reviewers,asuth

Revision D96583 was moved to bug 1676411. Setting attachment 9186914 [details] to obsolete.

Attachment #9186914 - Attachment is obsolete: true
Attachment #9186921 - Attachment is patch: true
Attachment #9186921 - Attachment mime type: application/octet-stream → text/plain

One more patch is coming to fix https://treeherder.mozilla.org/jobs?repo=try&revision=1ed76f7bd31a8f19fde9c97ec101fbc3cbbe136a&selectedTaskRun=XX_ZeaZQQdmLKCIrtapz4g.0
Basically, when the last private browsing window is closed we need to clear the data in the parent process as well (not only local storage caches in content processes).

(In reply to Jan Varga [:janv] from comment #12)

One more patch is coming to fix https://treeherder.mozilla.org/jobs?repo=try&revision=1ed76f7bd31a8f19fde9c97ec101fbc3cbbe136a&selectedTaskRun=XX_ZeaZQQdmLKCIrtapz4g.0
Basically, when the last private browsing window is closed we need to clear the data in the parent process as well (not only local storage caches in content processes).

I actually fixed that directly in https://phabricator.services.mozilla.com/D96562

Comment on attachment 9186843 [details]
Bug 1669437 - Add support for named in-memory databases; r=#dom-workers-and-storage-reviewers,asuth

Revision D96527 was moved to bug 1676573. Setting attachment 9186843 [details] to obsolete.

Attachment #9186843 - Attachment is obsolete: true
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/529cebcbb2b1
Get rid of the private data set; r=asuth,dom-workers-and-storage-reviewers
Attachment #9186885 - Attachment description: Bug 1669437 - Add support for independent in-memory only local storage database; r=#dom-workers-and-storage-reviewers,asuth → Bug 1669437 - Add necessary infrastructure for independent in-memory only local storage database; r=#dom-workers-and-storage-reviewers,asuth
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/661f0d8ae4c4
Add necessary infrastructure for independent in-memory only local storage database; r=asuth,dom-workers-and-storage-reviewers
Attachment #9186892 - Attachment description: Bug 1669437 - Enable synchronization of private browsing data to the parent process; r=#dom-workers-and-storage-reviewers,asuth → Bug 1669437 - Enable full sharing of private browsing data across content processes (using the in-memory database in the parent process); r=#dom-workers-and-storage-reviewers,asuth
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/565b1cd443cc
Enable full sharing of private browsing data across content processes (using the in-memory database in the parent process); r=asuth,dom-workers-and-storage-reviewers

https://hg.mozilla.org/mozilla-central/rev/529cebcbb2b1
https://hg.mozilla.org/mozilla-central/rev/661f0d8ae4c4

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox84: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

(In reply to Pulsebot from comment #17)

Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/661f0d8ae4c4
Add necessary infrastructure for independent in-memory only local storage
database; r=asuth,dom-workers-and-storage-reviewers

== Change summary for alert #27628 (as of Fri, 13 Nov 2020 02:43:28 GMT) ==

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new)
12% twitch LastVisualChange linux64-shippable-qr cold webrender 6,899.95 -> 6,059.17
4% pinterest loadtime linux64-shippable cold 2,155.74 -> 2,078.21

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

Flags: qe-verify? → qe-verify+

Reproduced the initial issue with protonmail using old Nightly from 2020-11-01, verified that for protonmail and Office365 this is fixed using Firefox 84.0b3 across platforms (Windows 10 64bit, macOS 11 and Ubuntu 18.04 64bit).

Status: RESOLVED → VERIFIED
status-firefox84: fixed → verified
Flags: qe-verify+

This issue is still present when you have "Delete cookies and site data when Firefox is closed" checked in preferences -> privacy & security -> cookies and site data of Firefox 86. Interestingly though, if you have this setting checked AND use a private browser window, the problem doesn't present.

Jan Varga, can the fix for private browser windows be applied when having the "Delete cookies and site data when Firefox is closed" option checked?

Any update on the issue (see comment above)?

Flags: needinfo?(jvarga)

The problem with cross-process session-only storage for legacy LocalStorage is tracked by bug 1453699. Bug 1681493 tracks moving from the current session-only storage/semantics where data never touches disk (and therefore leads to multi-process consistency problems) to using normal durable storage with data clearing at shutdown (which is actually what already happens for LocalStorage NextGen). https://bugzilla.mozilla.org/show_bug.cgi?id=1453699#c23 tracks the primary change that would be required in (legacy) LocalStorage to support that change (by the privacy team).

At the current moment, we've had good progress addressing the QuotaManager initialization failures that have prevented rolling out LocalStorage NextGen, so the most pragmatic course of action may be to go with the LSNG rollout.

Flags: needinfo?(jvarga)

At our team meeting today we revisited this scenario and we don't think we can enable LSNG for everyone in this release, so we have the following general plan:

  • For this release, implement bug 1703310 where we enable LSNG for the session-only userbase which is currently experiencing a very bad Legacy LS experience and for whom LSNG breakage (by way of QM breakage) is expected to be less likely and/or more likely to be able to take advantage of "Refresh Firefox" and support channels.
  • For this release or next release (we're unfortunately a little late in this release cycle to be sure of this), implement bug 1703317 which covers drastically improving the content breakage that results from LSNG breakage. (Also a problem in legacy LS when it breaks too...)
  • Continue to try and enable LSNG for all users, but that definitely will not happen during this release and we are optimistic for the next release.

The net result is that session-only users would be using LSNG as of Firefox 89 which will reach release on 2021-05-18.

That sounds like a great approach, I'll keep an eye on the progress of those two bugs. Thanks for the info Andrew!

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: