Last Comment Bug 722981 - nsMediaCacheFlusher uses the global Private Browsing status to decide when to flush the media cache
: nsMediaCacheFlusher uses the global Private Browsing status to decide when to...
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86 Mac OS X
-- normal (vote)
: mozilla15
Assigned To: :Ehsan Akhgari
: Maire Reavy [:mreavy] Please needinfo me
Depends on: 725210
Blocks: PBnGen
  Show dependency treegraph
Reported: 2012-01-31 22:03 PST by Josh Matthews [:jdm]
Modified: 2012-04-24 18:01 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (v1) (1.33 KB, patch)
2012-04-16 20:17 PDT, :Ehsan Akhgari
roc: review+
Details | Diff | Splinter Review

Description User image Josh Matthews [:jdm] 2012-01-31 22:03:09 PST
The global PB service is going away. We'll probably need to have two separate media caches, since there could be a PB window watching a video and a regular window watching another concurrently.
Comment 1 User image Josh Matthews [:jdm] 2012-02-08 10:46:50 PST
We can at least clear out the private media cache based on the last-pb-context-destroyed notification in bug 725210.
Comment 2 User image Josh Matthews [:jdm] 2012-04-15 21:50:23 PDT
Robert, what work will be required to allow multiple media caches? I scanned through nsMediaCache for places that use gMediaCache, and it didn't look like a completely straightforward process for someone brand new to the media code.
Comment 3 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2012-04-15 22:08:12 PDT
It's not trivial, but I think it's not hard.

-- Give each nsMediaCacheStream a reference to its nsMediaCache. Make nsMediaCacheStream::Init indicate which cache to use (could just be an enum). Then InitMediaCache would take that enum as a parameter and index into a global array. Or some value of the enum could create a new unique nsMediaCache and add it to the array.
-- Make MaybeShutdown nonstatic.
-- Make nsMediaCacheFlusher iterate through the global array. (Would it still be needed though?)
-- Give each ResourceStreamIterator a reference to its nsMediaCache.
-- Give each UpdateEvent a reference to its nsMediaCache.
Comment 4 User image :Ehsan Akhgari 2012-04-16 20:14:50 PDT
OK, so we had talked about this over in bug 572243.  To summarize, the media cache does not include the URL to the media resource.  The data itself might potentially be used to trace the media item back to its source, but the current approach of clearing the cache when the PB mode is over seems to be sufficient in order to defend against that.

The advantage of separating the media cache into two caches is that when leaving the PB mode, the media cache for non-PB media will not get evicted.  I think that's definitely something to strive to do, but it's out of scope for this bug.  As far as this bug is concerned, we should just preserve the existing functionality.
Comment 5 User image :Ehsan Akhgari 2012-04-16 20:17:14 PDT
Created attachment 615606 [details] [diff] [review]
Patch (v1)

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