Closed
Bug 1117808
Opened 10 years ago
Closed 3 years ago
Service Worker Cache should do the right thing in private browsing mode.
Categories
(Core :: Storage: Cache API, defect, P3)
Core
Storage: Cache API
Tracking
()
RESOLVED
DUPLICATE
of bug 1714354
People
(Reporter: bkelly, Unassigned)
References
Details
+++ This bug was initially created as a clone of Bug #1112134 +++
Splitting the discussion about private browsing mode off from bug 1112134.
Reporter | ||
Comment 1•10 years ago
|
||
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #12)
> (In reply to Ben Kelly (PTO, back Mon Jan 5) [:bkelly] from comment #11)
> > they can't be forensically recovered from the disk. If its between crashing
> > the whole browser from an OOM vs writing a temp file that might be
> > recoverable, do we really want to favor the OOM? (Keep in mind OOM'ing may
> > lose data in other non-PB tabs.)
>
> We should never favor an OOM, obviously. :-) Let's take a step back.
>
> How about this:
>
> 1. Ensure that if you access the cache API from within a private session, we
> will not return anything from the "normal" cache. That is, caches should
> give you the illusion that there is nothing stored from the origin initially.
Agreed. This should be required before pref'ing on.
> 2. Pretend that an origin that accesses the API from a private session has 0
> amount of storage left from the quota manager's perspective, and thus fail
> any request that tries to put anything into the cache.
>
> This will obviously make it possible to detect whether you are in PB mode,
> but I guess that ship has sailed with IDB (and of course with other side
> channel attacks that make it possible to detect this today.) But at least
> it keeps the same API surface exposed to content, so if (when) sites do
> stupid stuff such as assuming Firefox N+ has caches implemented, they won't
> break when loaded in PB mode.
Well, there is an open bug to make IDB pretend to function in bug 781982. Apparently people like to us PB mode to avoid caching during development and its annoying when IDB stops working as well. Chrome apparently keeps IDB working in this mode.
I think a short term solution would be to act like we have 0 bytes of storage. I would probably do this synthetically on the child side instead of trying to spin up the entire quota system in the parent.
Long term, I think we should investigate having quota manager create a memory backed storage area. I believe it has all the hooks necessary to do this:
- We use custom file URIs to launch sqlite with QM. This could be modified to create a memory-backed sqlite.
- QM could provide a custom nsIFile implementation that points to an in-memory structure.
- QM would similarly return memory-only file streams for those operations.
QM could then be configured to set a limit on the memory used for these structures.
Or maybe we could make memory-backed file systems on all platforms? I know you can do it on linux, but not sure about windows/mac.
Obviously this would be a longer term approach.
What do you guys think of these short term and long term solutions?
Flags: needinfo?(jonas)
Flags: needinfo?(ehsan)
Flags: needinfo?(Jan.Varga)
Comment 2•10 years ago
|
||
I like the short term approach!
What are the use cases for the long term approach though? Is it just the cache API? If yes, I doubt that it would be worth doing so much work to support it...
Flags: needinfo?(ehsan)
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #2)
> I like the short term approach!
>
> What are the use cases for the long term approach though? Is it just the
> cache API? If yes, I doubt that it would be worth doing so much work to
> support it...
The long term approach would solve the PB issue for all QuotaManager based APIs now and in the future. This includes IDB.
Let's debate the long-term solutions elsewhere. I think the short-term solution sounds good.
Flags: needinfo?(jonas)
Oh, I guess this bug was created as the "elsewhere" to debate the long-term solution :)
I don't know if a custom nsIFile implementation would really help. Things like the SQLite backend for IDB doesn't operate on nsIFile.
If we want storage to work in PB mode I think we need a real filesystem in order to keep the various storage APIs working.
A few potential solutions
* Have the OS create a memory back filesystem. I'm not sure if that would work on all of our tier-1
platforms?
* Create normal files, but then spin up a separate watcher process which deletes those files if Gecko
crashes. We already have the crash reporter doing something similar. Possibly we could reuse that
process? All we need is to delete a single directory.
* Have something similar to PB but specifically for developers. We could either simply have a checkbox
for "enable storage in PB mode" flag that developers could change. Or we could use the cookie-jar
support that gecko already has specifically when entering a "developer mode".
Reporter | ||
Comment 6•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #5)
> Oh, I guess this bug was created as the "elsewhere" to debate the long-term
> solution :)
>
> I don't know if a custom nsIFile implementation would really help. Things
> like the SQLite backend for IDB doesn't operate on nsIFile.
Right, but we already have a custom QM URI handler for opening an SQLite database. The QuotaManager could further customize this handler to use a memory-backed SQLite instance in this case. (SQLite already supports being memory-backed, etc.)
If you are doing file IO with QuotaManager, you must use the nsIFile provided to you by QM when you open the directory. You also have to use the QM-specific file streams. These are already requirements today.
It seems these existing hooks could just be extended to handle the memory-only case.
> If we want storage to work in PB mode I think we need a real filesystem in
> order to keep the various storage APIs working.
That would be ideal. I'm just not sure how possible it is.
(In reply to Ben Kelly [:bkelly] from comment #6)
> If you are doing file IO with QuotaManager, you must use the nsIFile
> provided to you by QM when you open the directory. You also have to use the
> QM-specific file streams. These are already requirements today.
I thought we were getting rid of those requirements in the new QM API.
Jan? Bent?
I'm very worried about trying to keep all this data in memory. That will make us OOM really fast on mobile. Both Fennec and B2G.
Flags: needinfo?(bent.mozilla)
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #7)
> I thought we were getting rid of those requirements in the new QM API.
Ah, well, I have not seen the new API yet. Maybe that indeed is coming.
> I'm very worried about trying to keep all this data in memory. That will
> make us OOM really fast on mobile. Both Fennec and B2G.
Well, I was also proposing that QM would then place a limit on it. Even a small limit would allow some use of the APIs for development purposes and to make it harder to detect PB mode.
I'm fine not taking on all this work. If we're going to ever fix bug 781982, though, it would be nice to do it at the QM layer so Cache and other code could share the benefit.
Yeah, I definitely think this needs to be solved at the QM level. We can't require all storage APIs to have a separate backend implementation for PB mode.
Comment 10•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #9)
> Yeah, I definitely think this needs to be solved at the QM level. We can't
> require all storage APIs to have a separate backend implementation for PB
> mode.
I support this approach too, ideally QM would provide a virtual file system API to its clients (just like SQLite does these days). That would open the door for other cool things like compressing data on the fly, encryption, etc.
Flags: needinfo?(Jan.Varga)
Comment 11•10 years ago
|
||
(In reply to Jan Varga [:janv] from comment #10)
> (In reply to Jonas Sicking (:sicking) from comment #9)
> > Yeah, I definitely think this needs to be solved at the QM level. We can't
> > require all storage APIs to have a separate backend implementation for PB
> > mode.
>
> I support this approach too, ideally QM would provide a virtual file system
> API to its clients (just like SQLite does these days). That would open the
> door for other cool things like compressing data on the fly, encryption, etc.
This sounds like a fine idea to me.
I would really rather we not do something so fancy as a virtual filesystem. It seems much easier to just have the QuotaManager hand out shmem/DELETE_ON_CLOSE FDs that can be wrapped into file streams or passed to SQLite or other storage backends, doesn't it?
Flags: needinfo?(bent.mozilla)
Comment 13•10 years ago
|
||
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #12)
> I would really rather we not do something so fancy as a virtual filesystem.
> It seems much easier to just have the QuotaManager hand out
> shmem/DELETE_ON_CLOSE FDs that can be wrapped into file streams or passed to
> SQLite or other storage backends, doesn't it?
We usually try not to write anything to the disk in private browsing mode in the first place, so it would be nice if we can get away without an fd backed by the disk even if it is DELETE_ON_CLOSE, but if not perhaps we can survive too...
(In reply to [Away: 1/29-2/20] from comment #13)
> We usually try not to write anything to the disk in private browsing mode
Yeah, definitely. But I doubt we can get away with that on mobile, and on desktop we'd have to have some smarts to handle large dbs...
Comment 15•10 years ago
|
||
Yeah, I doubt that there is an easy and ideal solution available here. :-)
Reporter | ||
Updated•9 years ago
|
Reporter | ||
Updated•9 years ago
|
Updated•6 years ago
|
Component: DOM → DOM: Service Workers
Priority: -- → P3
Updated•5 years ago
|
Component: DOM: Service Workers → Storage: Cache API
Comment 16•4 years ago
|
||
I'm interested in the follow up for this bug. I wonder if there is anyone we could reach out for championing the proposed fix.
Thanks!
Comment 17•3 years ago
|
||
Duping forward to bug 1714354 which reflects current plans.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•