Bug 49153 - Move Cache.* into loader/cache in as a start to cleaning up loader/
Summary: Move Cache.* into loader/cache in as a start to cleaning up loader/
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 49156
  Show dependency treegraph
 
Reported: 2010-11-07 19:33 PST by Eric Seidel (no email)
Modified: 2010-11-08 10:43 PST (History)
13 users (show)

See Also:


Attachments
Patch (8.31 KB, patch)
2010-11-07 19:35 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch (9.83 KB, patch)
2010-11-07 19:49 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
manually created patch file (9.25 KB, patch)
2010-11-07 19:53 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
created from an svn checkout (86.27 KB, patch)
2010-11-07 19:56 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-11-07 19:33:28 PST
Move Cache.* into loader/cache in as a start to cleaning up loader/
Comment 1 Eric Seidel (no email) 2010-11-07 19:35:28 PST
Created attachment 73203 [details]
Patch
Comment 2 Adam Barth 2010-11-07 19:39:46 PST
Comment on attachment 73203 [details]
Patch

This isn't going to compile, but yeah, this is what we want.
Comment 3 Eric Seidel (no email) 2010-11-07 19:42:04 PST
Attachment 73203 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5558013
Comment 4 Andreas Kling 2010-11-07 19:43:12 PST
Comment on attachment 73203 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=73203&action=review

> WebCore/WebCore.pro:1034
> -    loader/Cache.cpp \
> +    loader/cache/Cache.cpp \

You should do the same for loader/Cache.h in this file.
Comment 5 Early Warning System Bot 2010-11-07 19:45:55 PST
Attachment 73203 [details] did not build on qt:
Build output: http://queues.webkit.org/results/5516020
Comment 6 Eric Seidel (no email) 2010-11-07 19:49:16 PST
Created attachment 73204 [details]
Patch
Comment 7 Eric Seidel (no email) 2010-11-07 19:51:13 PST
It built on mac for me, not sure why the bot failed.
Comment 8 Eric Seidel (no email) 2010-11-07 19:51:45 PST
It built on mac for me, not sure why the bot failed.
Comment 9 Eric Seidel (no email) 2010-11-07 19:52:13 PST
It looks like the file moves are missing from the patch. I suspect Abarth's webkit-patch chagned_files optimization broke file moves.
Comment 10 Eric Seidel (no email) 2010-11-07 19:53:16 PST
Created attachment 73205 [details]
manually created patch file
Comment 11 Eric Seidel (no email) 2010-11-07 19:53:59 PST
it appears git diff does not include moved files?!
Comment 12 Eric Seidel (no email) 2010-11-07 19:54:42 PST
Attachment 73204 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5532024
Comment 13 Eric Seidel (no email) 2010-11-07 19:56:05 PST
Created attachment 73206 [details]
created from an svn checkout
Comment 14 Early Warning System Bot 2010-11-07 19:57:53 PST
Attachment 73204 [details] did not build on qt:
Build output: http://queues.webkit.org/results/5533023
Comment 15 Eric Seidel (no email) 2010-11-07 20:02:12 PST
Attachment 73203 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/5494017
Comment 16 Eric Seidel (no email) 2010-11-07 20:09:30 PST
Committed r71492: <http://trac.webkit.org/changeset/71492>
Comment 17 Eric Seidel (no email) 2010-11-07 20:11:06 PST
Attachment 73205 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5553023
Comment 18 Alexey Proskuryakov 2010-11-07 21:37:49 PST
Eric, why is this bug with a patch pending review in RESOLVED/FIXED state?
Comment 19 Eric Seidel (no email) 2010-11-07 21:39:48 PST
Probably because webkit-patch land doesn't clear r?

I don't use webkit-patch land much, maybe clearing of r? broke (or never was?)
Comment 20 Adam Barth 2010-11-07 21:45:38 PST
Maybe because it only clears r+ ?
Comment 21 Darin Adler 2010-11-08 10:19:28 PST
The biggest problem with the Cache class right now is that it’s not just a cache. Moving it into a directory named cache makes that problem worse.
Comment 22 Adam Barth 2010-11-08 10:43:27 PST
(In reply to comment #21)
> The biggest problem with the Cache class right now is that it’s not just a cache. Moving it into a directory named cache makes that problem worse.

What else does it do besides being a cache?  It's involved in the loading process too, but I was planning to move that logic out into a separate class in the interests of making the main document cachable in the memory cache.