Closed Bug 516013 Opened 11 years ago Closed 11 years ago

Aggressive caching of the current lightweight theme

Categories

(Toolkit :: Add-ons Manager, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: dao, Assigned: dao)

References

Details

(Keywords: verified1.9.2)

Attachments

(1 file)

Right now the images for lightweight themes use the web cache, but we should store them more robustly for users who clear the cache on shutdown or servers that send bad caching headers, and maybe other cases.
OS: Windows XP → All
Hardware: x86 → All
Flags: blocking1.9.2+
Dao, I believe you said this was on your plate, not Dave's, so I'm assigning it to you to clear out the "blockers assigned to nobody@" list. If I'm wrong, please feel free to reassign. I don't have to tell you how important it is that we get this piece working.
Assignee: nobody → dao
Another common case is users who establish a network connection after starting their browser (indeed, often because they start their browser, and its first request initiates the network connection attempt).  In that case, we won't be able to load the images from the network on startup, and perhaps not from the web cache either (even when the images are in it).
Priority: -- → P2
Attached patch WIPSplinter Review
This works, I'm not sure that it's sane...
Comment on attachment 403499 [details] [diff] [review]
WIP

I think dumping unknown files in the profile folder could be dangerous, but maybe it's not. Dave, what do you think?
Attachment #403499 - Flags: review?(dtownsend)
I don't think storing the files in the profile is a particular problem, after all the images are probably getting stored in the cache in the profile right now anyway. I wonder whether we should consider caching the files for all recent themes though. This would solve the problem with trying to change to a different theme when offline.
I was trying to avoid inventing a file structure, e.g. theme -> file name mappings (just using the IDs as is doesn't seem right, as they aren't sanitized), and keeping track of these files. And while it's unfortunate that you can't switch themes in offline mode, I don't think that's all that important. As a quick and dirty fix, the Add-ons manager could probably hide any lightweight themes except for the current one in offline mode.
Attachment #403499 - Flags: review?(dtownsend) → review-
Comment on attachment 403499 [details] [diff] [review]
WIP

I think we need to verify that aStatus is a success and that the http status was a successful one or we'll end up caching an error page.

I'm also wondering if there is an easy way we can kick off persisting the images in the event that the user exits before it is done.
(In reply to comment #7)
> (From update of attachment 403499 [details] [diff] [review])
> I think we need to verify that aStatus is a success and that the http status
> was a successful one or we'll end up caching an error page.

requestSucceeded seems to translate to status==200.
Comment on attachment 403499 [details] [diff] [review]
WIP

Missed that in my read through, this is fine.
Attachment #403499 - Flags: review- → review+
http://hg.mozilla.org/mozilla-central/rev/5f03363ae12d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
http://hg.mozilla.org/mozilla-central/rev/f02075968efe
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Depends on: 522188
dao, how can i test this?
Cut your network connection, clear the cache and restart Firefox.
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091202 Minefield/3.7a1pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b5pre) Gecko/20091202 Namoroka/3.6b5pre
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
You need to log in before you can comment on or make changes to this bug.