Closed
Bug 1455723
Opened 6 years ago
Closed 5 years ago
Firefox59 does not properly honor cache size set in autoconfig files
Categories
(Core :: Networking: Cache, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla67
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: jmp242, Assigned: michal)
References
Details
(Whiteboard: [necko-triaged])
Attachments
(1 file, 1 obsolete file)
42.79 KB,
patch
|
mayhemer
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Build ID: 20180319000641 Steps to reproduce: Create an autoconfig file with "browser.cache.disk.capacity": { "value": 24576 }, "browser.cache.disk.smart_size.enabled": { "value": false }, Actual results: Cache size shown in UI is 1024 MB Expected results: Cache size shown in UI and used is 24MB
Comment 1•6 years ago
|
||
So it appears the issue is when you make it a default pref, it gets set to a different calue. If you make it a user pref or a locked pref, it shoudld work properly.
Please correct as needed if this is the incorrect component. Thanks.
Comment 3•6 years ago
|
||
Taking for now because I want to debug before it's passed along.
Assignee: nobody → mozilla
Updated•6 years ago
|
Priority: -- → P3
Comment 4•6 years ago
|
||
https://searchfox.org/mozilla-central/source/netwerk/cache/nsCacheService.cpp#672 In order to switch users over to smart caching, the code checks to see if the user ever had set a value and if not, assume we can switch them. That's why it works when you set a user value. Since the default smart size value is true, this code needs to be modified so that if smartsize is set to false, the firstRun code isn't followed at all.
Comment 5•6 years ago
|
||
Mike, do we still care about that?
Flags: needinfo?(mozilla)
Summary: FirefoxESR52 and RR59 do not properly honor cache size set in autoconfig files → Firefox59 does not properly honor cache size set in autoconfig files
Updated•6 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 7•6 years ago
|
||
Assignee | ||
Comment 8•6 years ago
|
||
Honza, shouldn't we deny all prefs manipulation in the old cache code when cache2 is used?
Flags: needinfo?(honzab.moz)
Comment 9•6 years ago
|
||
(In reply to Michal Novotny [:michal] from comment #8) > Honza, shouldn't we deny all prefs manipulation in the old cache code when > cache2 is used? Oh, definitely. I don't think there is anymore a way to even use cache1, except appcache, which has its own set of preferences and doesn't touch this particular bug at all. OTOH, we still have the smart cache size concept even in the new cache, right? Is there a different pref to manipulate it that is expected to work but it isn't? Should we fix something in the new cache code (CacheObserver, I assume) then?
Flags: needinfo?(honzab.moz) → needinfo?(michal.novotny)
Assignee | ||
Comment 10•6 years ago
|
||
Bug 1382688 removed possibility to use the old disk cache. New cache uses the same preference to enable/disable smart cache size and the old cache shouldn't touch it. I'll fix it.
Assignee: mozilla → michal.novotny
Component: File Handling → Networking: Cache
Flags: needinfo?(michal.novotny)
Product: Firefox → Core
Comment 11•6 years ago
|
||
(In reply to Michal Novotny [:michal] from comment #10) > Bug 1382688 removed possibility to use the old disk cache. New cache uses > the same preference to enable/disable smart cache size and the old cache > shouldn't touch it. I'll fix it. OK, if the prefs are the same, then the autoconfig file should work. But it seems like it doesn't. Hence, it looks like we need to fix something, but not in the cache1 code, rather in the cache2 code? That was the point of my questions.
Assignee | ||
Comment 12•6 years ago
|
||
Did you read the comments in phabricator? The problem is that prefs are read and set in the old cache, so this has to be removed.
Comment 13•6 years ago
|
||
(In reply to Michal Novotny [:michal] from comment #12) > Did you read the comments in phabricator? The problem is that prefs are read > and set in the old cache, so this has to be removed. Why are people writing comments to phab and not here? OK, I finally get it! If the patch prevents the Set*Pref calls that offend the autoconf file, let's take it. Thanks for explanation.
Assignee | ||
Comment 14•5 years ago
|
||
Removes all preferences manipulation from old cache code that isn't used by offline cache. It removes also some related code (e.g. everything smart size related, unused defines etc.), but the goal wasn't to remove all unused code from the old cache.
Attachment #9022722 -
Attachment is obsolete: true
Attachment #9036321 -
Flags: review?(honzab.moz)
Assignee | ||
Comment 15•5 years ago
|
||
Comment 16•5 years ago
|
||
Comment on attachment 9036321 [details] [diff] [review] fix Review of attachment 9036321 [details] [diff] [review]: ----------------------------------------------------------------- thanks and sorry for the delay.
Attachment #9036321 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Updated•5 years ago
|
Keywords: checkin-needed
Comment 17•5 years ago
|
||
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/780eec2d27c3
Firefox59 does not properly honor cache size set in autoconfig files, r=mayhemer
Keywords: checkin-needed
Comment 18•5 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox67:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Updated•5 years ago
|
Attachment #9022722 -
Attachment is obsolete: false
Updated•5 years ago
|
Attachment #9022722 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•