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 Bugs

Assignee:

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:

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	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
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.