54683 2011-02-17 12:56:51 -0800 WebKit2: The CFNetwork Cache should be shared between the UI Process and the Web Process on Windows 2011-02-17 14:26:00 -0800 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) All Windows 7 RESOLVED FIXED InRadar, PlatformOnly P2 Normal --- 1 jberlin jberlin aroben jberlin sfalken oldest_to_newest 353124 0 jberlin 2011-02-17 12:56:51 -0800 There is already code to share the CFNetwork Cache between the UI Process and the Web Process on Mac. We should do the same on Windows. <rdar://problem/8882342> 353126 1 82852 jberlin 2011-02-17 13:00:35 -0800 Created attachment 82852 Patch 353129 2 82852 aroben 2011-02-17 13:03:52 -0800 Comment on attachment 82852 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82852&action=review > Source/WebKit2/ChangeLog:22 > + Add the parameters for the path to the cache, the disk capacity, and the memory capacity: > + * Shared/WebProcessCreationParameters.cpp: > + (WebKit::WebProcessCreationParameters::encode): > + (WebKit::WebProcessCreationParameters::decode): > + * Shared/WebProcessCreationParameters.h: > + > + * UIProcess/win/WebContextWin.cpp: > + (WebKit::WebContext::platformInitializeWebProcess): > + Make sure to remove the ending slash, as CFNetwork does not recognize the directory with > + that slash. > + > + * WebProcess/win/WebProcessWin.cpp: > + (WebKit::WebProcess::platformInitializeWebProcess): > + Create a cache using the path, disk capacity, and memory capacity and set it as default. Having some comments above the items they apply to and some below confuses me! > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:72 > encoder->encode(shouldPaintNativeControls); > + encoder->encode(cfURLCachePath); > + encoder->encode(cfURLCacheMemoryCapacity); > + encoder->encode(cfURLCacheDiskCapacity); It would be nice to match the order in which these are declared in the struct. > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:128 > if (!decoder->decode(parameters.shouldPaintNativeControls)) > return false; > + if (!decoder->decode(parameters.cfURLCachePath)) > + return false; > + if (!decoder->decode(parameters.cfURLCacheMemoryCapacity)) > + return false; > + if (!decoder->decode(parameters.cfURLCacheDiskCapacity)) > + return false; Ditto. > Source/WebKit2/UIProcess/win/WebContextWin.cpp:64 > + parameters.cfURLCachePath = wkCopyFoundationCacheDirectory(); Presumably wkCopyFoundationCacheDirectory returns a CFStringRef, which you're leaking? 353166 3 jberlin 2011-02-17 14:14:13 -0800 (In reply to comment #2) > (From update of attachment 82852 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82852&action=review > > > Source/WebKit2/ChangeLog:22 > > + Add the parameters for the path to the cache, the disk capacity, and the memory capacity: > > + * Shared/WebProcessCreationParameters.cpp: > > + (WebKit::WebProcessCreationParameters::encode): > > + (WebKit::WebProcessCreationParameters::decode): > > + * Shared/WebProcessCreationParameters.h: > > + > > + * UIProcess/win/WebContextWin.cpp: > > + (WebKit::WebContext::platformInitializeWebProcess): > > + Make sure to remove the ending slash, as CFNetwork does not recognize the directory with > > + that slash. > > + > > + * WebProcess/win/WebProcessWin.cpp: > > + (WebKit::WebProcess::platformInitializeWebProcess): > > + Create a cache using the path, disk capacity, and memory capacity and set it as default. > > Having some comments above the items they apply to and some below confuses me! Removed the comment above the items it applied to, since it was not really helpful anyways. > > > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:72 > > encoder->encode(shouldPaintNativeControls); > > + encoder->encode(cfURLCachePath); > > + encoder->encode(cfURLCacheMemoryCapacity); > > + encoder->encode(cfURLCacheDiskCapacity); > > It would be nice to match the order in which these are declared in the struct. Done. > > > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:128 > > if (!decoder->decode(parameters.shouldPaintNativeControls)) > > return false; > > + if (!decoder->decode(parameters.cfURLCachePath)) > > + return false; > > + if (!decoder->decode(parameters.cfURLCacheMemoryCapacity)) > > + return false; > > + if (!decoder->decode(parameters.cfURLCacheDiskCapacity)) > > + return false; > > Ditto. Done. > > > Source/WebKit2/UIProcess/win/WebContextWin.cpp:64 > > + parameters.cfURLCachePath = wkCopyFoundationCacheDirectory(); > > Presumably wkCopyFoundationCacheDirectory returns a CFStringRef, which you're leaking? Whoops! Fixed. Thanks for the review! 353175 4 82852 jberlin 2011-02-17 14:25:49 -0800 Comment on attachment 82852 Patch Committed in r78916 http://trac.webkit.org/changeset/78916 82852 2011-02-17 13:00:35 -0800 2011-02-17 14:25:49 -0800 Patch makeTheWebProcessUseTheSameCacheAsUIProcess54683.patch text/plain 5296 jberlin SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDc4ODg3KQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjYgQEAKKzIwMTEtMDItMTcgIEplc3NpZSBC ZXJsaW4gIDxqYmVybGluQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBXZWJLaXQyOiBUaGUgQ0ZOZXR3b3JrIENhY2hlIHNob3VsZCBi ZSBzaGFyZWQgYmV0d2VlbiB0aGUgVUkgUHJvY2VzcyBhbmQgdGhlIFdlYiBQcm9jZXNzIG9uCisg ICAgICAgIFdpbmRvd3MuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD01NDY4MworCisgICAgICAgIEFkZCB0aGUgcGFyYW1ldGVycyBmb3IgdGhlIHBhdGgg dG8gdGhlIGNhY2hlLCB0aGUgZGlzayBjYXBhY2l0eSwgYW5kIHRoZSBtZW1vcnkgY2FwYWNpdHk6 CisgICAgICAgICogU2hhcmVkL1dlYlByb2Nlc3NDcmVhdGlvblBhcmFtZXRlcnMuY3BwOgorICAg ICAgICAoV2ViS2l0OjpXZWJQcm9jZXNzQ3JlYXRpb25QYXJhbWV0ZXJzOjplbmNvZGUpOgorICAg ICAgICAoV2ViS2l0OjpXZWJQcm9jZXNzQ3JlYXRpb25QYXJhbWV0ZXJzOjpkZWNvZGUpOgorICAg ICAgICAqIFNoYXJlZC9XZWJQcm9jZXNzQ3JlYXRpb25QYXJhbWV0ZXJzLmg6CisgCisgICAgICAg ICogVUlQcm9jZXNzL3dpbi9XZWJDb250ZXh0V2luLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2Vi Q29udGV4dDo6cGxhdGZvcm1Jbml0aWFsaXplV2ViUHJvY2Vzcyk6CisgICAgICAgIE1ha2Ugc3Vy ZSB0byByZW1vdmUgdGhlIGVuZGluZyBzbGFzaCwgYXMgQ0ZOZXR3b3JrIGRvZXMgbm90IHJlY29n bml6ZSB0aGUgZGlyZWN0b3J5IHdpdGgKKyAgICAgICAgdGhhdCBzbGFzaC4KKworICAgICAgICAq IFdlYlByb2Nlc3Mvd2luL1dlYlByb2Nlc3NXaW4uY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJQ cm9jZXNzOjpwbGF0Zm9ybUluaXRpYWxpemVXZWJQcm9jZXNzKToKKyAgICAgICAgQ3JlYXRlIGEg Y2FjaGUgdXNpbmcgdGhlIHBhdGgsIGRpc2sgY2FwYWNpdHksIGFuZCBtZW1vcnkgY2FwYWNpdHkg YW5kIHNldCBpdCBhcyBkZWZhdWx0LgorCiAyMDExLTAyLTE3ICBCcmlhbiBXZWluc3RlaW4gIDxi d2VpbnN0ZWluQGFwcGxlLmNvbT4KIAogICAgICAgICBRdCBidWlsZCBmaXguCkluZGV4OiBTb3Vy Y2UvV2ViS2l0Mi9TaGFyZWQvV2ViUHJvY2Vzc0NyZWF0aW9uUGFyYW1ldGVycy5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYktpdDIvU2hhcmVkL1dlYlByb2Nlc3NDcmVhdGlvblBhcmFtZXRl cnMuY3BwCShyZXZpc2lvbiA3ODgzNikKKysrIFNvdXJjZS9XZWJLaXQyL1NoYXJlZC9XZWJQcm9j ZXNzQ3JlYXRpb25QYXJhbWV0ZXJzLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNjcsNiArNjcsOSBA QCB2b2lkIFdlYlByb2Nlc3NDcmVhdGlvblBhcmFtZXRlcnM6OmVuY29kCiAgICAgZW5jb2Rlci0+ ZW5jb2RlKHVpUHJvY2Vzc0J1bmRsZVJlc291cmNlUGF0aCk7CiAjZWxpZiBQTEFURk9STShXSU4p CiAgICAgZW5jb2Rlci0+ZW5jb2RlKHNob3VsZFBhaW50TmF0aXZlQ29udHJvbHMpOworICAgIGVu Y29kZXItPmVuY29kZShjZlVSTENhY2hlUGF0aCk7CisgICAgZW5jb2Rlci0+ZW5jb2RlKGNmVVJM Q2FjaGVNZW1vcnlDYXBhY2l0eSk7CisgICAgZW5jb2Rlci0+ZW5jb2RlKGNmVVJMQ2FjaGVEaXNr Q2FwYWNpdHkpOwogI2VuZGlmCiB9CiAKQEAgLTExNyw2ICsxMjAsMTIgQEAgYm9vbCBXZWJQcm9j ZXNzQ3JlYXRpb25QYXJhbWV0ZXJzOjpkZWNvZAogI2VsaWYgUExBVEZPUk0oV0lOKQogICAgIGlm ICghZGVjb2Rlci0+ZGVjb2RlKHBhcmFtZXRlcnMuc2hvdWxkUGFpbnROYXRpdmVDb250cm9scykp CiAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICBpZiAoIWRlY29kZXItPmRlY29kZShwYXJhbWV0 ZXJzLmNmVVJMQ2FjaGVQYXRoKSkKKyAgICAgICAgcmV0dXJuIGZhbHNlOworICAgIGlmICghZGVj b2Rlci0+ZGVjb2RlKHBhcmFtZXRlcnMuY2ZVUkxDYWNoZU1lbW9yeUNhcGFjaXR5KSkKKyAgICAg ICAgcmV0dXJuIGZhbHNlOworICAgIGlmICghZGVjb2Rlci0+ZGVjb2RlKHBhcmFtZXRlcnMuY2ZV UkxDYWNoZURpc2tDYXBhY2l0eSkpCisgICAgICAgIHJldHVybiBmYWxzZTsKICNlbmRpZgogCiAg ICAgcmV0dXJuIHRydWU7CkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9TaGFyZWQvV2ViUHJvY2Vzc0Ny ZWF0aW9uUGFyYW1ldGVycy5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1NoYXJlZC9X ZWJQcm9jZXNzQ3JlYXRpb25QYXJhbWV0ZXJzLmgJKHJldmlzaW9uIDc4ODM2KQorKysgU291cmNl L1dlYktpdDIvU2hhcmVkL1dlYlByb2Nlc3NDcmVhdGlvblBhcmFtZXRlcnMuaAkod29ya2luZyBj b3B5KQpAQCAtODUsNiArODUsMTAgQEAgc3RydWN0IFdlYlByb2Nlc3NDcmVhdGlvblBhcmFtZXRl cnMgewogCiAgICAgQ1N0cmluZyB1aVByb2Nlc3NCdW5kbGVSZXNvdXJjZVBhdGg7CiAjZWxpZiBQ TEFURk9STShXSU4pCisgICAgdWludDY0X3QgY2ZVUkxDYWNoZU1lbW9yeUNhcGFjaXR5OworICAg IHVpbnQ2NF90IGNmVVJMQ2FjaGVEaXNrQ2FwYWNpdHk7CisgICAgU3RyaW5nIGNmVVJMQ2FjaGVQ YXRoOworCiAgICAgYm9vbCBzaG91bGRQYWludE5hdGl2ZUNvbnRyb2xzOwogI2VuZGlmCiB9OwpJ bmRleDogU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL3dpbi9XZWJDb250ZXh0V2luLmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mvd2luL1dlYkNvbnRleHRXaW4uY3Bw CShyZXZpc2lvbiA3ODgzNikKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy93aW4vV2ViQ29u dGV4dFdpbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMwLDYgKzMwLDExIEBACiAjaW5jbHVkZSAi V2ViUHJvY2Vzc01lc3NhZ2VzLmgiCiAjaW5jbHVkZSA8V2ViQ29yZS9GaWxlU3lzdGVtLmg+CiAK KyNpZiBVU0UoQ0ZORVRXT1JLKQorI2luY2x1ZGUgPENGTmV0d29yay9DRlVSTENhY2hlUHJpdi5o PgorI2luY2x1ZGUgPFdlYktpdFN5c3RlbUludGVyZmFjZS9XZWJLaXRTeXN0ZW1JbnRlcmZhY2Uu aD4gCisjZW5kaWYKKwogdXNpbmcgbmFtZXNwYWNlIFdlYkNvcmU7CiAKIG5hbWVzcGFjZSBXZWJL aXQgewpAQCAtNTEsNiArNTYsMTUgQEAgdm9pZCBXZWJDb250ZXh0OjpzZXRTaG91bGRQYWludE5h dGl2ZUNvbgogdm9pZCBXZWJDb250ZXh0OjpwbGF0Zm9ybUluaXRpYWxpemVXZWJQcm9jZXNzKFdl YlByb2Nlc3NDcmVhdGlvblBhcmFtZXRlcnMmIHBhcmFtZXRlcnMpCiB7CiAgICAgcGFyYW1ldGVy cy5zaG91bGRQYWludE5hdGl2ZUNvbnRyb2xzID0gbV9zaG91bGRQYWludE5hdGl2ZUNvbnRyb2xz OworCisjaWYgVVNFKENGTkVUV09SSykKKyAgICBSZXRhaW5QdHI8Q0ZVUkxDYWNoZVJlZj4gY2Z1 cmxDYWNoZShBZG9wdENGLCBDRlVSTENhY2hlQ29weVNoYXJlZFVSTENhY2hlKCkpOworICAgIHBh cmFtZXRlcnMuY2ZVUkxDYWNoZU1lbW9yeUNhcGFjaXR5ID0gQ0ZVUkxDYWNoZU1lbW9yeUNhcGFj aXR5KGNmdXJsQ2FjaGUuZ2V0KCkpOworICAgIHBhcmFtZXRlcnMuY2ZVUkxDYWNoZURpc2tDYXBh Y2l0eSA9IENGVVJMQ2FjaGVEaXNrQ2FwYWNpdHkoY2Z1cmxDYWNoZS5nZXQoKSk7CisgICAgcGFy YW1ldGVycy5jZlVSTENhY2hlUGF0aCA9IHdrQ29weUZvdW5kYXRpb25DYWNoZURpcmVjdG9yeSgp OworICAgIC8vIFJlbW92ZSB0aGUgZW5kaW5nICcvJyAobmVjZXNzYXJ5IHRvIGhhdmUgQ0ZOZXR3 b3JrIGZpbmQgdGhlIENhY2hlIGZpbGUpLgorICAgIHBhcmFtZXRlcnMuY2ZVUkxDYWNoZVBhdGgu cmVtb3ZlKHBhcmFtZXRlcnMuY2ZVUkxDYWNoZVBhdGgubGVuZ3RoKCkgLSAxKTsKKyNlbmRpZgog fQogCiBTdHJpbmcgV2ViQ29udGV4dDo6cGxhdGZvcm1EZWZhdWx0RGF0YWJhc2VEaXJlY3Rvcnko KSBjb25zdApJbmRleDogU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy93aW4vV2ViUHJvY2Vzc1dp bi5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy93aW4vV2ViUHJv Y2Vzc1dpbi5jcHAJKHJldmlzaW9uIDc4ODM2KQorKysgU291cmNlL1dlYktpdDIvV2ViUHJvY2Vz cy93aW4vV2ViUHJvY2Vzc1dpbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEwOCw2ICsxMDgsMTcg QEAgdm9pZCBXZWJQcm9jZXNzOjpwbGF0Zm9ybUNsZWFyUmVzb3VyY2VDYQogdm9pZCBXZWJQcm9j ZXNzOjpwbGF0Zm9ybUluaXRpYWxpemVXZWJQcm9jZXNzKGNvbnN0IFdlYlByb2Nlc3NDcmVhdGlv blBhcmFtZXRlcnMmIHBhcmFtZXRlcnMsIENvcmVJUEM6OkFyZ3VtZW50RGVjb2RlciopCiB7CiAg ICAgc2V0U2hvdWxkUGFpbnROYXRpdmVDb250cm9scyhwYXJhbWV0ZXJzLnNob3VsZFBhaW50TmF0 aXZlQ29udHJvbHMpOworCisjaWYgVVNFKENGTkVUV09SSykKKyAgICBSZXRhaW5QdHI8Q0ZTdHJp bmdSZWY+IGNhY2hlUGF0aChBZG9wdENGLCBwYXJhbWV0ZXJzLmNmVVJMQ2FjaGVQYXRoLmNyZWF0 ZUNGU3RyaW5nKCkpOworICAgIGlmICghY2FjaGVQYXRoKQorICAgICAgICByZXR1cm47CisKKyAg ICBDRkluZGV4IGNhY2hlTWVtb3J5Q2FwYWNpdHkgPSBwYXJhbWV0ZXJzLmNmVVJMQ2FjaGVNZW1v cnlDYXBhY2l0eTsKKyAgICBDRkluZGV4IGNhY2hlRGlza0NhcGFjaXR5ID0gcGFyYW1ldGVycy5j ZlVSTENhY2hlRGlza0NhcGFjaXR5OworICAgIFJldGFpblB0cjxDRlVSTENhY2hlUmVmPiB1aVBy b2Nlc3NDYWNoZShBZG9wdENGLCBDRlVSTENhY2hlQ3JlYXRlKGtDRkFsbG9jYXRvckRlZmF1bHQs IGNhY2hlTWVtb3J5Q2FwYWNpdHksIGNhY2hlRGlza0NhcGFjaXR5LCBjYWNoZVBhdGguZ2V0KCkp KTsKKyAgICBDRlVSTENhY2hlU2V0U2hhcmVkVVJMQ2FjaGUodWlQcm9jZXNzQ2FjaGUuZ2V0KCkp OworI2VuZGlmCiB9CiAKIHZvaWQgV2ViUHJvY2Vzczo6cGxhdGZvcm1TaHV0ZG93bigpCg==