1167106 - [Woodduck][FFOS2.0]Browser cache will exceed 55MB limit

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[ZHANG Jin]

Attachment: image004.jpg

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.104 Safari/537.36

Steps to reproduce:

Keep browsering and watch size of cache partition.


Actual results:

the 100MB cache partition will be full after browsering lots of websites


Expected results:

cache files should take no more than 55MB as set in b2g.js

#ifdef MOZ_WIDGET_GONK
pref("browser.cache.disk.enable", true);
pref("browser.cache.disk.capacity", 55000); // kilobytes
pref("browser.cache.disk.parent_directory", "/cache");
#endif

Comment 1

[Kai-Zhen Li [:seinlin][:kli]]

How do you watch the cache partition? Can you share your details?

Comment 2

[Kai-Zhen Li [:seinlin][:kli]]

Sorry for the typo! 

Actually, what I mean is your detail steps of watching cache partition.

Comment 3

[ZHANG Jin]

use "adb shell df" to check partition size

Filesystem               Size     Used     Free   Blksize
/system                140.7M   124.6M    16.1M   4096
/data                   66.9M    35.0M    31.9M   4096
/cache                 100.0M     5.6M    94.4M   4096
/custpack               67.9M    57.9M    10.0M   4096

Comment 4

[ZHANG Jin]

Attachment: Device parameters got from webIDE

here "brower.cache.disk.capacity" set to 101376, why & where this pref been overrided?

Comment 5

[wei.deng]

NI Kai-Zhen Li
Hi Kai-Zhen,
if the /cache space has been fulled by browser's cache or other apps cache.end user haven't any way to clear it, and my question is what risk will be happened in this status? would you please help to anwser? thanks a lot.

Comment 6

[ZHANG Jin]

Seems somehow we got a smart cache size here according to size of cache partition. There should be a limit, or our FOTA function will not work as there's no room for update packages. We are adding a wipe-cache function before FOTA, but the disk capacity pref should work as well.

Comment 7

[ZHANG Jin]

hi kli,

according to https://dxr.mozilla.org/mozilla-central/source/netwerk/cache/nsCacheService.cpp#658,
>>           // If user explicitly set cache size to be smaller than old default
>>            // of 50 MB, then keep user's value. Otherwise use smart sizing.

seems if we set "browser.cache.disk.capacity" to less than 50MB, things will work.  Would you please give us a proper value on Wooduck(512M ROM: 140M system;100M cache;67M data) ? 

Thx

Comment 8

[ZHANG Jin]

root cause found

https://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#1098
set pref here..

Comment 9

[ZHANG Jin]

another question,

any risk if our update package takes, say 20M in /cache, leaves only 80M for browser, but brower requests a 99M capacity?

Comment 10

[Kai-Zhen Li [:seinlin][:kli]]

brower.cache.disk.capacity will be overridden at [1]. The reason is not all Devices have the same size for /cache partition, so it need to be adjusted according to the real size of /cache partition.

https://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#1106

Comment 11

[Kai-Zhen Li [:seinlin][:kli]]

Attachment: bug-1167106.patch

Hi Fabrice, 

I think we need to adjust 'browser.cache.disk.capacity' only when the default value [1] is larger than the physical /cache partition size. If the physical size is enough we should keep default value. This can keep the flexibility of customization per requirement.

How do you think? 

I verified this patch does work properly. If you agree, I'll submit a formal patch for landing.

Thanks!

[1] https://dxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#50

Comment 12

[[:fabrice] Fabrice Desré]

Comment on attachment 8608891 [details] [diff] [review]
bug-1167106.patch

Review of attachment 8608891 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, good catch. We added this code for devices where the cache partition was smaller than 55MB...

Comment 13

[Josh Cheng [:josh]]

Hi Kai-Zhen,
Could you help to land this on 2.0M? Thanks!

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/3a1b9e32ba2b

Comment 15

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/0ef86d9dd93b

Comment 16

[Kai-Zhen Li [:seinlin][:kli]]

Comment on attachment 8608891 [details] [diff] [review]
bug-1167106.patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: browser will always use cache as much as the partition size, this will effect other features which need cache.
Testing completed: Verified on master and 2.0m.
Risk to taking this patch (and alternatives if risky): Low, just update the logic to select cache size.
String or UUID changes made by this patch: none

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3a1b9e32ba2b

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0cf413ec1966

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/b8e024cd73a5

Comment 20

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/b8e024cd73a5