Bug 49153 – Move Cache.* into loader/cache in as a start to cleaning up loader/

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Move Cache.* into loader/cache in as a start to cleaning up loader/

[Eric Seidel (no email)]

Created attachment 73203 [details]
Patch

[Adam Barth]

Comment on attachment 73203 [details]
Patch

This isn't going to compile, but yeah, this is what we want.

[Eric Seidel (no email)]

Attachment 73203 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5558013

[Andreas Kling]

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]

Attachment 73203 [details] did not build on qt:
Build output: http://queues.webkit.org/results/5516020

[Eric Seidel (no email)]

Created attachment 73204 [details]
Patch

[Eric Seidel (no email)]

It built on mac for me, not sure why the bot failed.

[Eric Seidel (no email)]

It built on mac for me, not sure why the bot failed.

[Eric Seidel (no email)]

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)]

Created attachment 73205 [details]
manually created patch file

[Eric Seidel (no email)]

it appears git diff does not include moved files?!

[Eric Seidel (no email)]

Attachment 73204 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5532024

[Eric Seidel (no email)]

Created attachment 73206 [details]
created from an svn checkout

[Early Warning System Bot]

Attachment 73204 [details] did not build on qt:
Build output: http://queues.webkit.org/results/5533023

[Eric Seidel (no email)]

Attachment 73203 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/5494017

[Eric Seidel (no email)]

Committed r71492: <http://trac.webkit.org/changeset/71492>

[Eric Seidel (no email)]

Attachment 73205 [details] did not build on mac:
Build output: http://queues.webkit.org/results/5553023

[Alexey Proskuryakov]

Eric, why is this bug with a patch pending review in RESOLVED/FIXED state?

[Eric Seidel (no email)]

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]

Maybe because it only clears r+ ?

[Darin Adler]

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]

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