190912 2018-10-25 11:46:47 -0700 http/wpt/cache-storage/cache-quota.any.html is failing 2018-10-25 21:43:49 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED DUPLICATE 178316 P2 Normal --- 1 dkadu webkit-unassigned bugs-noreply fred.wang mcatanzaro oldest_to_newest 1472138 0 dkadu 2018-10-25 11:46:47 -0700 http/wpt/cache-storage/cache-quota.any.html test is failing, This can be fixed by setting maximum cache quota limit in testcontroller of gtk 1472139 1 353098 dkadu 2018-10-25 11:57:00 -0700 Created attachment 353098 Patch 1472146 2 353098 fred.wang 2018-10-25 12:13:37 -0700 Comment on attachment 353098 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353098&action=review You need to remove the failures from LayoutTests/platform/gtk/TestExpectations and LayoutTests/platform/wpe/TestExpectations. Let's do that on bug 178316 directly. > Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h:116 > +WK_EXPORT void WKWebsiteDataStoreSetQuota(WKWebsiteDataStoreRef dataStoreRef, uint64_t quota); I'm not familiar with the rules to introduce new API, so let's see what other reviewers say. > Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:117 > + WKWebsiteDataStoreSetQuota(websiteDataStore, 400 * 1024); I think you should do the same for WPE. 1472147 3 fred.wang 2018-10-25 12:14:05 -0700 *** This bug has been marked as a duplicate of bug 178316 *** 1472192 4 353098 mcatanzaro 2018-10-25 14:57:36 -0700 Comment on attachment 353098 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353098&action=review > Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:577 > +void WKWebsiteDataStoreSetQuota(WKWebsiteDataStoreRef dataStoreRef, uint64_t quota) > +{ > + WebKit::toImpl(dataStoreRef)->websiteDataStore().setCacheStoragePerOriginQuota(quota); > +} This isn't good naming, because it's extremely unclear what quota is being set. You wouldn't know from the name of the function that it's only setting the cache storage quota. I'm also concerned this doesn't match the Mac API, which is called _setCacheStoragePerOriginQuota. So the solution is to name this function WKWebsiteDataStoreSetCacheStoragePerOriginQuota. When we have plumbing like this, with several layers of similar API calls, it's important to keep the naming consistent or else reading the code becomes very confusing. >> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h:116 >> +WK_EXPORT void WKWebsiteDataStoreSetQuota(WKWebsiteDataStoreRef dataStoreRef, uint64_t quota); > > I'm not familiar with the rules to introduce new API, so let's see what other reviewers say. It's internal API for use by the TestController. No rules other than that an owner (e.g. Youenn in this case) must approve. Remember the C API is not actually public or stable. > Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:31 > +#include "UIProcess/API/C/WKWebsiteDataStoreRef.h" #include <WebKit/WKWebsiteDataStoreRef.h> If it's not in the include path (should already be), then it should be added in CMakeLists.txt. >> Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:117 >> + WKWebsiteDataStoreSetQuota(websiteDataStore, 400 * 1024); > > I think you should do the same for WPE. This should be done in the main TestController.cpp, not here. Then remove the Mac-specific API call from TestControllerCocoa.mm. And then remove the Mac-specific API entirely (from WKWebsiteDataStorePrivate.h and WKWebsiteDataStore.mm), since it's only used in one place by TestController and there's nothing platform-specific about it. 1472326 5 fred.wang 2018-10-25 21:43:49 -0700 (In reply to Michael Catanzaro from comment #4) > >> Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:117 > >> + WKWebsiteDataStoreSetQuota(websiteDataStore, 400 * 1024); > > > > I think you should do the same for WPE. > > This should be done in the main TestController.cpp, not here. Then remove > the Mac-specific API call from TestControllerCocoa.mm. And then remove the > Mac-specific API entirely (from WKWebsiteDataStorePrivate.h and > WKWebsiteDataStore.mm), since it's only used in one place by TestController > and there's nothing platform-specific about it. I agree with Michael and we had already discussed with Darshan about moving to the main TestController class. Since he is new to WebKit, I think it's ok to start with just the minimal changes to make tests pass on WPE/GTK and then do follow-up refactoring so that the quota limit is set on all platforms at one place. 353098 2018-10-25 11:57:00 -0700 2018-10-25 12:13:37 -0700 Patch bug-190912-20181026002645.patch text/plain 4100 dkadu U3VidmVyc2lvbiBSZXZpc2lvbjogMjM2NjQ0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDZjMGRhNTUyYmIwYmNkOGMz MzNhMzgxNWU1ZGRjNWVmN2E1OTVjZWQuLmQxYmE5ODc3ZGNmZWNkNDg0YmUyMjQ0ZWEyZjMwZWU5 ZTVjOTAyZTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTgtMTAtMjUgIERhcnNoYW4g S2FkdSAgPGRhcnNoNzgwN0BnbWFpbC5jb20+CisKKyAgICAgICAgaHR0cC93cHQvY2FjaGUtc3Rv cmFnZS9jYWNoZS1xdW90YS5hbnkuaHRtbCBpcyBmYWlsaW5nCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTA5MTIKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQy9XS1dlYnNpdGVE YXRhU3RvcmVSZWYuY3BwOgorICAgICAgICAoV0tXZWJzaXRlRGF0YVN0b3JlU2V0UXVvdGEpOgor ICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvQy9XS1dlYnNpdGVEYXRhU3RvcmVSZWYuaDoKKwogMjAx OC0wOS0yOSAgQ29tbWl0IFF1ZXVlICA8Y29tbWl0LXF1ZXVlQHdlYmtpdC5vcmc+CiAKICAgICAg ICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjIzNjYzMS4KZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJLaXQvVUlQcm9jZXNzL0FQSS9DL1dLV2Vic2l0ZURhdGFTdG9yZVJlZi5jcHAgYi9Tb3VyY2Uv V2ViS2l0L1VJUHJvY2Vzcy9BUEkvQy9XS1dlYnNpdGVEYXRhU3RvcmVSZWYuY3BwCmluZGV4IDJl ZTUzZDRhY2MwN2JkNTMxNTQ4NTQ4YzljZGM0NjRlYWNlMTMxNjEuLmU3ZDJlNGNhNTMyY2VlOGVj NTA5ZDIyMDhmNGM4MzA3NWRkMTE2NzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9j ZXNzL0FQSS9DL1dLV2Vic2l0ZURhdGFTdG9yZVJlZi5jcHAKKysrIGIvU291cmNlL1dlYktpdC9V SVByb2Nlc3MvQVBJL0MvV0tXZWJzaXRlRGF0YVN0b3JlUmVmLmNwcApAQCAtNTcxLDYgKzU3MSwx MCBAQCB2b2lkIFdLV2Vic2l0ZURhdGFTdG9yZVNldFNlcnZpY2VXb3JrZXJSZWdpc3RyYXRpb25E aXJlY3RvcnkoV0tXZWJzaXRlRGF0YVN0b3JlUgogICAgIFdlYktpdDo6dG9JbXBsKGRhdGFTdG9y ZVJlZiktPndlYnNpdGVEYXRhU3RvcmUoKS5zZXRTZXJ2aWNlV29ya2VyUmVnaXN0cmF0aW9uRGly ZWN0b3J5KFdlYktpdDo6dG9JbXBsKHNlcnZpY2VXb3JrZXJSZWdpc3RyYXRpb25EaXJlY3Rvcnkp LT5zdHJpbmcoKSk7CiB9CiAKK3ZvaWQgV0tXZWJzaXRlRGF0YVN0b3JlU2V0UXVvdGEoV0tXZWJz aXRlRGF0YVN0b3JlUmVmIGRhdGFTdG9yZVJlZiwgdWludDY0X3QgcXVvdGEpCit7CisgICAgV2Vi S2l0Ojp0b0ltcGwoZGF0YVN0b3JlUmVmKS0+d2Vic2l0ZURhdGFTdG9yZSgpLnNldENhY2hlU3Rv cmFnZVBlck9yaWdpblF1b3RhKHF1b3RhKTsgCit9CiAKIHZvaWQgV0tXZWJzaXRlRGF0YVN0b3Jl U2V0V2ViQXV0aGVudGljYXRpb25Nb2NrQ29uZmlndXJhdGlvbihXS1dlYnNpdGVEYXRhU3RvcmVS ZWYgZGF0YVN0b3JlUmVmLCBXS0RpY3Rpb25hcnlSZWYgY29uZmlndXJhdGlvblJlZikKIHsKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0FQSS9DL1dLV2Vic2l0ZURhdGFTdG9y ZVJlZi5oIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0MvV0tXZWJzaXRlRGF0YVN0b3Jl UmVmLmgKaW5kZXggZWNlOGEzNWFiODhkMzI4MGIyNjMzYzVhM2ViODRlYmYxNzNiZTEyYy4uMmM3 NmQ0N2IwZTU4ODNkMmYxNTNjYTBlMDliNjQxNzJhZDk5MWNlOSAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdC9VSVByb2Nlc3MvQVBJL0MvV0tXZWJzaXRlRGF0YVN0b3JlUmVmLmgKKysrIGIvU291 cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0MvV0tXZWJzaXRlRGF0YVN0b3JlUmVmLmgKQEAgLTEx Myw3ICsxMTMsNyBAQCBXS19FWFBPUlQgdm9pZCBXS1dlYnNpdGVEYXRhU3RvcmVHZXRGZXRjaENh Y2hlU2l6ZUZvck9yaWdpbihXS1dlYnNpdGVEYXRhU3RvcmVSZQogCiBXS19FWFBPUlQgV0tTdHJp bmdSZWYgV0tXZWJzaXRlRGF0YVN0b3JlQ29weVNlcnZpY2VXb3JrZXJSZWdpc3RyYXRpb25EaXJl Y3RvcnkoV0tXZWJzaXRlRGF0YVN0b3JlUmVmIGRhdGFTdG9yZVJlZik7CiBXS19FWFBPUlQgdm9p ZCBXS1dlYnNpdGVEYXRhU3RvcmVTZXRTZXJ2aWNlV29ya2VyUmVnaXN0cmF0aW9uRGlyZWN0b3J5 KFdLV2Vic2l0ZURhdGFTdG9yZVJlZiBkYXRhU3RvcmVSZWYsIFdLU3RyaW5nUmVmIHNlcnZpY2VX b3JrZXJSZWdpc3RyYXRpb25EaXJlY3RvcnkpOwotCitXS19FWFBPUlQgdm9pZCBXS1dlYnNpdGVE YXRhU3RvcmVTZXRRdW90YShXS1dlYnNpdGVEYXRhU3RvcmVSZWYgZGF0YVN0b3JlUmVmLCB1aW50 NjRfdCBxdW90YSk7CiBXS19FWFBPUlQgdm9pZCBXS1dlYnNpdGVEYXRhU3RvcmVTZXRXZWJBdXRo ZW50aWNhdGlvbk1vY2tDb25maWd1cmF0aW9uKFdLV2Vic2l0ZURhdGFTdG9yZVJlZiBkYXRhU3Rv cmVSZWYsIFdLRGljdGlvbmFyeVJlZiBjb25maWd1cmF0aW9uKTsKIAogI2lmZGVmIF9fY3BsdXNw bHVzCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cgYi9Ub29scy9DaGFuZ2VMb2cKaW5kZXgg NDc3YzRkZjUwN2ExMTRhNjZiYmE0MmU1NjQyM2IwOTM0YzY0OWY1ZC4uZDdiMmJmNWNiYjA5ZDM1 MWQzMDkwNjdkMTJjY2FkODdkYmM1MjUzZCAxMDA2NDQKLS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisr KyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE4LTEwLTI1ICBEYXJzaGFu IEthZHUgIDxkYXJzaDc4MDdAZ21haWwuY29tPgorCisgICAgICAgIGh0dHAvd3B0L2NhY2hlLXN0 b3JhZ2UvY2FjaGUtcXVvdGEuYW55Lmh0bWwgaXMgZmFpbGluZworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkwOTEyCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBXZWJLaXRUZXN0UnVubmVyL2d0ay9UZXN0 Q29udHJvbGxlckd0ay5jcHA6CisgICAgICAgIChXVFI6OlRlc3RDb250cm9sbGVyOjpwbGF0Zm9y bUluaXRpYWxpemVDb250ZXh0KToKKwogMjAxOC0wOS0yOSAgQ29tbWl0IFF1ZXVlICA8Y29tbWl0 LXF1ZXVlQHdlYmtpdC5vcmc+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjIz NjYzMS4KZGlmZiAtLWdpdCBhL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvZ3RrL1Rlc3RDb250cm9s bGVyR3RrLmNwcCBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvZ3RrL1Rlc3RDb250cm9sbGVyR3Rr LmNwcAppbmRleCA0ZjJlOGM0M2E1M2IwZmQ5ODEwMzE1Mjk5MDI1NGJkNGVlNjhjMzUyLi4xZTY1 MTU3MWEyYjE2ZDU1YzM3ZWYxMDgwNjRhZmMwNTVkZjI0YTVkIDEwMDY0NAotLS0gYS9Ub29scy9X ZWJLaXRUZXN0UnVubmVyL2d0ay9UZXN0Q29udHJvbGxlckd0ay5jcHAKKysrIGIvVG9vbHMvV2Vi S2l0VGVzdFJ1bm5lci9ndGsvVGVzdENvbnRyb2xsZXJHdGsuY3BwCkBAIC0yOCw2ICsyOCw3IEBA CiAjaW5jbHVkZSAiVGVzdENvbnRyb2xsZXIuaCIKIAogI2luY2x1ZGUgIlBsYXRmb3JtV2ViVmll dy5oIgorI2luY2x1ZGUgIlVJUHJvY2Vzcy9BUEkvQy9XS1dlYnNpdGVEYXRhU3RvcmVSZWYuaCIK ICNpbmNsdWRlIDxndGsvZ3RrLmg+CiAjaW5jbHVkZSA8d3RmL1BsYXRmb3JtLmg+CiAjaW5jbHVk ZSA8d3RmL1J1bkxvb3AuaD4KQEAgLTExMiw2ICsxMTMsOCBAQCB2b2lkIFRlc3RDb250cm9sbGVy Ojppbml0aWFsaXplVGVzdFBsdWdpbkRpcmVjdG9yeSgpCiAKIHZvaWQgVGVzdENvbnRyb2xsZXI6 OnBsYXRmb3JtSW5pdGlhbGl6ZUNvbnRleHQoKQogeworICAgIGF1dG8qIHdlYnNpdGVEYXRhU3Rv cmUgPSBXS0NvbnRleHRHZXRXZWJzaXRlRGF0YVN0b3JlKHBsYXRmb3JtQ29udGV4dCgpKTsKKyAg ICBXS1dlYnNpdGVEYXRhU3RvcmVTZXRRdW90YSh3ZWJzaXRlRGF0YVN0b3JlLCA0MDAgKiAxMDI0 KTsKIH0KIAogdm9pZCBUZXN0Q29udHJvbGxlcjo6c2V0SGlkZGVuKGJvb2wgaGlkZGVuKQo=