Bug 173361 - [Win] DRT should not use disk cache.
Summary: [Win] DRT should not use disk cache.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords:
Depends on:
Blocks: 173399
  Show dependency treegraph
 
Reported: 2017-06-14 03:55 PDT by Per Arne Vollan
Modified: 2018-07-31 12:45 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.07 KB, patch)
2017-06-14 03:57 PDT, Per Arne Vollan
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2017-06-14 03:55:49 PDT
Like on macOS/iOS, DRT on Windows should only use memory cache.
Comment 1 Per Arne Vollan 2017-06-14 03:57:41 PDT
Created attachment 312883 [details]
Patch
Comment 2 Darin Adler 2017-06-14 07:37:20 PDT
Comment on attachment 312883 [details]
Patch

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

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1469
> +    RetainPtr<CFURLCacheRef> newCache = adoptCF(CFURLCacheCreate(kCFAllocatorDefault, 1024 * 1024, 0, nullptr));

I suggest using auto here, or just putting this inside the function call.
Comment 3 Per Arne Vollan 2017-06-14 07:52:14 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 312883 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312883&action=review
> 
> > Tools/DumpRenderTree/win/DumpRenderTree.cpp:1469
> > +    RetainPtr<CFURLCacheRef> newCache = adoptCF(CFURLCacheCreate(kCFAllocatorDefault, 1024 * 1024, 0, nullptr));
> 
> I suggest using auto here, or just putting this inside the function call.

Thanks for reviewing! I will update the patch before landing.
Comment 4 Per Arne Vollan 2017-06-14 07:58:28 PDT
Committed r218255: <https://trac.webkit.org/changeset/218255/webkit>
Comment 5 Basuke Suzuki 2018-07-26 15:44:56 PDT
Per Arne,
What is the source of this decision? We have an cache implementation in WebKitLegacy and wanna test it. So I am very curious what causes this patch.
Comment 6 Per Arne Vollan 2018-07-31 09:26:43 PDT
(In reply to Basuke Suzuki from comment #5)
> Per Arne,
> What is the source of this decision? We have an cache implementation in
> WebKitLegacy and wanna test it. So I am very curious what causes this patch.

I believe we turned off the disk cache because macOS turned it off. You can probably enable it for WinCairo to test the cache implementation.
Comment 7 Basuke Suzuki 2018-07-31 12:45:49 PDT
(In reply to Per Arne Vollan from comment #6)
> (In reply to Basuke Suzuki from comment #5)
> > Per Arne,
> > What is the source of this decision? We have an cache implementation in
> > WebKitLegacy and wanna test it. So I am very curious what causes this patch.

Can you check AppleWin support disk cache or not? Does CFNetwork itself has disk cache feature? I guess that's why there's no need to test'em explicitly with DRT.


> I believe we turned off the disk cache because macOS turned it off. You can
> probably enable it for WinCairo to test the cache implementation.

Yes, we are thinking about that. The code was old and we don't currently maintain them well, but without test, we can't touch them.