Bug 49153

Summary: Move Cache.* into loader/cache in as a start to cleaning up loader/
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, darin, dbates, dglazkov, fishd, kling, mitz, steveblock, tkent, webkit-ews, xan.lopez, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 49156    
Attachments:
Description Flags
Patch
none
Patch
none
manually created patch file
none
created from an svn checkout none

Eric Seidel (no email)
Reported 2010-11-07 19:33:28 PST
Move Cache.* into loader/cache in as a start to cleaning up loader/
Attachments
Patch (8.31 KB, patch)
2010-11-07 19:35 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (9.83 KB, patch)
2010-11-07 19:49 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
manually created patch file (9.25 KB, patch)
2010-11-07 19:53 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
created from an svn checkout (86.27 KB, patch)
2010-11-07 19:56 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-11-07 19:35:28 PST
Created attachment 73203 [details] Patch
Adam Barth
Comment 2 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.
Eric Seidel (no email)
Comment 3 2010-11-07 19:42:04 PST
Attachment 73203 [details] did not build on mac: Build output: http://queues.webkit.org/results/5558013
Andreas Kling
Comment 4 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.
Early Warning System Bot
Comment 5 2010-11-07 19:45:55 PST
Attachment 73203 [details] did not build on qt: Build output: http://queues.webkit.org/results/5516020
Eric Seidel (no email)
Comment 6 2010-11-07 19:49:16 PST
Created attachment 73204 [details] Patch
Eric Seidel (no email)
Comment 7 2010-11-07 19:51:13 PST
It built on mac for me, not sure why the bot failed.
Eric Seidel (no email)
Comment 8 2010-11-07 19:51:45 PST
It built on mac for me, not sure why the bot failed.
Eric Seidel (no email)
Comment 9 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.
Eric Seidel (no email)
Comment 10 2010-11-07 19:53:16 PST
Created attachment 73205 [details] manually created patch file
Eric Seidel (no email)
Comment 11 2010-11-07 19:53:59 PST
it appears git diff does not include moved files?!
Eric Seidel (no email)
Comment 12 2010-11-07 19:54:42 PST
Attachment 73204 [details] did not build on mac: Build output: http://queues.webkit.org/results/5532024
Eric Seidel (no email)
Comment 13 2010-11-07 19:56:05 PST
Created attachment 73206 [details] created from an svn checkout
Early Warning System Bot
Comment 14 2010-11-07 19:57:53 PST
Attachment 73204 [details] did not build on qt: Build output: http://queues.webkit.org/results/5533023
Eric Seidel (no email)
Comment 15 2010-11-07 20:02:12 PST
Attachment 73203 [details] did not build on chromium: Build output: http://queues.webkit.org/results/5494017
Eric Seidel (no email)
Comment 16 2010-11-07 20:09:30 PST
Committed r71492: <http://trac.webkit.org/changeset/71492>
Eric Seidel (no email)
Comment 17 2010-11-07 20:11:06 PST
Attachment 73205 [details] did not build on mac: Build output: http://queues.webkit.org/results/5553023
Alexey Proskuryakov
Comment 18 2010-11-07 21:37:49 PST
Eric, why is this bug with a patch pending review in RESOLVED/FIXED state?
Eric Seidel (no email)
Comment 19 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?)
Adam Barth
Comment 20 2010-11-07 21:45:38 PST
Maybe because it only clears r+ ?
Darin Adler
Comment 21 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.
Adam Barth
Comment 22 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.
Note You need to log in before you can comment on or make changes to this bug.