Closed
Bug 961024
Opened 12 years ago
Closed 11 years ago
Decide where to store HTTP cache v2 data on Android
Categories
(Core :: Networking: Cache, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: mayhemer, Assigned: mayhemer)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file, 1 obsolete file)
6.95 KB,
patch
|
michal
:
review+
mfinkle
:
feedback+
|
Details | Diff | Splinter Review |
Based on advice from bsmith we currently store all cache data (300MB+) in the Android's context cache directory, see [1].
According https://bugzilla.mozilla.org/show_bug.cgi?id=921142#c2 however it seems the reasonable limit is only 1MB for this cache dir content and there is no auto-eviction from OS at all.
So, also as we already have a (close to land) code for intelligent eviction of our own, should we rather move the HTTP cache storage to local profile, as the current cache (v1) does?
[1] http://hg.mozilla.org/mozilla-central/annotate/b53589696cf8/netwerk/cache2/CacheFileIOManager.cpp#l948
![]() |
Assignee | |
Updated•12 years ago
|
Flags: needinfo?(mark.finkle)
Comment 1•12 years ago
|
||
I think we should consider using the Android context cache as a start, although I would like a refresher on the prs/cons of doing that. I thought at one time that Android did not count that space against your "Size on Disk" footprint.
I am concerned that Android would flip out if we start putting 250MB worth of data in there, but we still might want to try it and see.
If we see weird issues, we can always move it back to the profile. Wait... profiles!
We have a Guest Browsing mode that uses a separate profile under the covers. The Android context cache does not have a concept of profiles. Therefore, I think we need to use the existing profile based system.
NI'ing Brian to confirm my suspicion.
Flags: needinfo?(mark.finkle) → needinfo?(bnicholson)
Comment 2•12 years ago
|
||
It would be nice to use the default cache location for a couple of reasons:
1) The "Clear cache" button in the app settings does the right thing and clears the cache
2) The cache is automatically cleared for us if we're running out of space
I'm not sure whether the cache counts toward the reported app size or not; if not, that's another bonus.
(In reply to Honza Bambas (:mayhemer) from comment #0)
> According https://bugzilla.mozilla.org/show_bug.cgi?id=921142#c2 however it
> seems the reasonable limit is only 1MB for this cache dir content and there
> is no auto-eviction from OS at all.
I think the 1MB given in the Android docs is just an arbitrary limit they picked as an example; reading the docs, the impression I get is that the cache size limit is up for the app developer to decide.
I agree with the profile concerns Mark mentioned. Even though Fennec isn't multi-profile now (excluding guest mode), it would be good to plan for it since we're building this from the ground up anyway. Could we add dirs in the context cache directories that match the Fennec profile names? That is, if Fennec has profiles X, Y, and Z, can we create X, Y, and Z subdirs in the context cache directory to avoid conflicts?
Flags: needinfo?(bnicholson)
![]() |
Assignee | |
Comment 3•12 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #2)
> I agree with the profile concerns Mark mentioned. Even though Fennec isn't
> multi-profile now (excluding guest mode), it would be good to plan for it
> since we're building this from the ground up anyway. Could we add dirs in
> the context cache directories that match the Fennec profile names? That is,
> if Fennec has profiles X, Y, and Z, can we create X, Y, and Z subdirs in the
> context cache directory to avoid conflicts?
Definitely can. I just need the info which if the profiles (X, Y or Z) is being used.
Question is if android also deletes files in subdirs, (the cache is even now in subdisrs, so no blocker for profile distinction). According what the cache size info shows when viewing Firefox app status in settings, it seems like android is at least aware of files even in subdirs.
![]() |
Assignee | |
Comment 4•12 years ago
|
||
Mark, how can I recognize the profile dir used?
Flags: needinfo?(mark.finkle)
Comment 5•12 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #4)
> Mark, how can I recognize the profile dir used?
You could could getDir("ProfD") and pulloff the leaf folder name. That's probably easier that using nsIToolkitProfileService.selectedProfile and getting the folder (as an nsIFile). Either way, I think you need to grab the leaf folder from an nsIFile.
Flags: needinfo?(mark.finkle)
![]() |
||
Comment 6•11 years ago
|
||
I received some comments related to a blog post about the (old) cache on Firefox for Android: http://gbrownmozilla.wordpress.com/2013/06/19/firefox-for-androids-disk-cache/#comments. Some people are frustrated that the cache (and profile) is on /data, rather than /sdcard -- something to keep in mind.
![]() |
Assignee | |
Updated•11 years ago
|
Blocks: cache2afterenable
![]() |
Assignee | |
Comment 7•11 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #6)
> I received some comments related to a blog post about the (old) cache on
> Firefox for Android:
> http://gbrownmozilla.wordpress.com/2013/06/19/firefox-for-androids-disk-
> cache/#comments. Some people are frustrated that the cache (and profile) is
> on /data, rather than /sdcard -- something to keep in mind.
The http cache storage location can be altered with browser.cache.disk.parent_directory pref. This can be exposed somewhere to UI with drop down options like /data, /cache, /sdcard or |custom|.
![]() |
Assignee | |
Updated•11 years ago
|
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 8•11 years ago
|
||
- on android (widget) we add to the CACHE_DIRECTORY env (when set) name of the profile (ProfD leaf native name)
Attachment #8428719 -
Flags: review?(mark.finkle)
Comment 9•11 years ago
|
||
Comment on attachment 8428719 [details] [diff] [review]
v1
Review of attachment 8428719 [details] [diff] [review]:
-----------------------------------------------------------------
If I read this correctly, this will start a new cache directory in the new location, but leave the old (~200M dir) where it currently is, further bloating the profile so FatFennec can now use 400M of storage?
Comment 10•11 years ago
|
||
Comment on attachment 8428719 [details] [diff] [review]
v1
This looks like it should create folders in the AppCache based on the current profile.
I have the same question as Gian-Carlo: What do we need to do to cleanup the existing cache2 folder in the AppCache? Do we think that Android will just cleanup all the files eventually (since it's the AppCache)?
Comment 11•11 years ago
|
||
>What do we need to do to cleanup the existing cache2 folder in the AppCache?
It's not in the AppCache right now, is it?
![]() |
Assignee | |
Comment 12•11 years ago
|
||
By "AppCache" you mean the html5 offline application cache? What that should have in common with HTTP cache location?
According cleanup, that is tricky, but we do some cleanup magic in bug 1014394 around it, so it could be used for any old mobile cache data wipe out as well.
Depends on: 1014394
Comment 13•11 years ago
|
||
>By "AppCache" you mean the html5 offline application cache? What that should have in common with HTTP cache location?
The Android temporary storage you get when you call getCacheDir(): http://developer.android.com/guide/topics/data/data-storage.html#filesInternal and which is shown as "Cache" when the user looks at the application stats.
It has the interesting property that files there will magically disappear when Android is low on space.
As far as I know, this is only used by nsDeleteDir so far, not cache2 itself.
![]() |
Assignee | |
Comment 14•11 years ago
|
||
(In reply to Gian-Carlo Pascutto [:gcp] from comment #13)
> >By "AppCache" you mean the html5 offline application cache? What that should have in common with HTTP cache location?
>
> The Android temporary storage you get when you call getCacheDir()
Ah, this cache. It has so many names :)
>
> As far as I know, this is only used by nsDeleteDir so far, not cache2 itself.
No, cache2 is using it too: http://hg.mozilla.org/mozilla-central/annotate/e017c15325ae/netwerk/cache2/CacheFileIOManager.cpp#l1343
The code in nsDeleteDir should probably be removed so that upper levels take care to set the dir explicitly. I have to check on that code once more.
![]() |
Assignee | |
Comment 15•11 years ago
|
||
- apply on top of bug 1017546
- as in v1, we inject profile dir leaf name between the patch and "cache2"
- this patch also deletes cache2 + cache2.Trash* located in the root of the android cache dir (so called "profileless" directory)
- I'm keeping the profileless dir at a new member IOMan::mCacheProfilelessDirectory, I personally don't want to parse mCacheDirecory, seems to me potentially unreliable
Attachment #8428719 -
Attachment is obsolete: true
Attachment #8428719 -
Flags: review?(mark.finkle)
Attachment #8431162 -
Flags: review?(michal.novotny)
Attachment #8431162 -
Flags: feedback?(mark.finkle)
Comment 16•11 years ago
|
||
Comment on attachment 8431162 [details] [diff] [review]
v2
This looks OK to me. I wish we didn't need to put the extra code in for the "profileless" directory, but that will clean up the current cache2 folder.
Can we remove the extra "profileless" code in Fx33? As far as I know the "profileless" cache2 folder will only exist on Fx32 Nightly. It should not exist on Fx31 (Aurora) right? I mean, once this patch lands :)
Attachment #8431162 -
Flags: feedback?(mark.finkle) → feedback+
![]() |
Assignee | |
Comment 17•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #16)
> Comment on attachment 8431162 [details] [diff] [review]
> v2
>
> This looks OK to me. I wish we didn't need to put the extra code in for the
> "profileless" directory, but that will clean up the current cache2 folder.
>
> Can we remove the extra "profileless" code in Fx33? As far as I know the
> "profileless" cache2 folder will only exist on Fx32 Nightly. It should not
> exist on Fx31 (Aurora) right? I mean, once this patch lands :)
Filed bug 1018192 on it.
![]() |
Assignee | |
Comment 18•11 years ago
|
||
Comment on attachment 8431162 [details] [diff] [review]
v2
Review of attachment 8431162 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/cache2/CacheStorageService.cpp
@@ +593,5 @@
> nsDeleteDir::RemoveOldTrashes(mCache2Dir);
> }
> +#if defined(MOZ_WIDGET_ANDROID)
> + if (mCache2Profileless) {
> + nsDeleteDir::RemoveOldTrashes(mCache2Profileless);
this RemoveOldTrashes call can be removed since with MOZ_WIDGET_ANDROID the trash dirs are always looked for in the root of the android cache directory. So, it's actually redundant with nsDeleteDir::RemoveOldTrashes call in the if (mCache2Dir) branch.
Updated•11 years ago
|
Attachment #8431162 -
Flags: review?(michal.novotny) → review+
Comment 19•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•