61255 2011-05-22 16:46:48 -0700 [Windows] Initial Cookie Accept Policy not set on Deserialized Storage Session 2011-05-23 10:36:33 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) All Windows 7 RESOLVED FIXED InRadar P2 Normal --- 1 jberlin jberlin darin jberlin sfalken oldest_to_newest 407715 0 jberlin 2011-05-22 16:46:48 -0700 <rdar://problem/9483885> 407716 1 94357 jberlin 2011-05-22 16:55:46 -0700 Created attachment 94357 Patch 407757 2 94357 darin 2011-05-22 22:08:09 -0700 Comment on attachment 94357 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94357&action=review > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:144 > + WebCookieManager::shared().setHTTPCookieAcceptPolicy(parameters.initialHTTPCookieAcceptPolicy); Moving this one line up seems like the bug fix. The other changes seem OK, but I don’t see why they have to go along with this bug fix in the same patch. They could be done separately before or after. > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:147 > + // By using the default storage session that came from the UI Process, the Web Process > + // automatically use the same the URL Cache as UI Process. I suggest not capitalizing terms such as “web process”. Also, the second line should read: // automatically uses the same URL cache as the UI process. 407758 3 darin 2011-05-22 22:08:25 -0700 r=me whether you decide to break this up in pieces or not 408026 4 jberlin 2011-05-23 10:14:57 -0700 (In reply to comment #2) > (From update of attachment 94357 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94357&action=review > > > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:144 > > + WebCookieManager::shared().setHTTPCookieAcceptPolicy(parameters.initialHTTPCookieAcceptPolicy); > > Moving this one line up seems like the bug fix. The other changes seem OK, but I don’t see why they have to go along with this bug fix in the same patch. They could be done separately before or after. It is true they could be done separately, but I without them the code with my change is harder to read (which was my motivation for making those changes now). I think I am going to keep them (per your other comment) for that reason. > > > Source/WebKit2/WebProcess/win/WebProcessWin.cpp:147 > > + // By using the default storage session that came from the UI Process, the Web Process > > + // automatically use the same the URL Cache as UI Process. > > I suggest not capitalizing terms such as “web process”. Also, the second line should read: > > // automatically uses the same URL cache as the UI process. Fixed. Thanks for the review! 408051 5 94357 jberlin 2011-05-23 10:36:24 -0700 Comment on attachment 94357 Patch Committed in http://trac.webkit.org/changeset/87077 94357 2011-05-22 16:55:46 -0700 2011-05-23 10:36:24 -0700 Patch setInitialCookieAcceptPolicy.patch text/plain 2719 jberlin SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDg3MDQ2KQorKysgU291cmNlL1dlYktpdDIvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDUtMjIgIEplc3NpZSBC ZXJsaW4gIDxqYmVybGluQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBbV2luZG93c10gSW5pdGlhbCBDb29raWUgQWNjZXB0IFBvbGlj eSBub3Qgc2V0IG9uIERlc2VyaWFsaXplZCBTdG9yYWdlIFNlc3Npb24uCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MTI1NQorCisgICAgICAgICogV2Vi UHJvY2Vzcy93aW4vV2ViUHJvY2Vzc1dpbi5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYlByb2Nl c3M6OnBsYXRmb3JtSW5pdGlhbGl6ZVdlYlByb2Nlc3MpOgorICAgICAgICBNb3ZlIHRoZSBjb2Rl IHRvIHNldCB0aGUgY29va2llIHBvbGljeSBiZWZvcmUgdGhlIGVhcmx5IHJldHVybiB0aGF0IGlz IHRha2VuIHdoZW4gdXNpbmcgdGhlCisgICAgICAgIGRlZmF1bHQgc3RvcmFnZSBzZXNzaW9uIGZy b20gdGhlIFVJIFByb2Nlc3MuCisgICAgICAgIEFsc28sIHJlbW92ZSB1bm5lY2Vzc2FyeSAjaWZk ZWZzIChVU0UoQ0ZVUkxTVE9SQUdFU0VTU0lPTlMpIGlzIHRydWUgaWYgVVNFKENGTkVUV09SSykg aXMKKyAgICAgICAgdHJ1ZSkgYW5kIGFkZCBhIGNvbW1lbnQgZXhwbGFpbmluZyB0aGUgZWFybHkg cmV0dXJuLgorCiAyMDExLTA1LTIxICBKb2huIFN1bGxpdmFuICA8c3VsbGl2YW5AYXBwbGUuY29t PgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpJbmRleDogU291cmNlL1dlYktp dDIvV2ViUHJvY2Vzcy93aW4vV2ViUHJvY2Vzc1dpbi5jcHAKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy93aW4vV2ViUHJvY2Vzc1dpbi5jcHAJKHJldmlzaW9uIDg3MDQz KQorKysgU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy93aW4vV2ViUHJvY2Vzc1dpbi5jcHAJKHdv cmtpbmcgY29weSkKQEAgLTEzOCwxMiArMTM4LDE1IEBAIHZvaWQgV2ViUHJvY2Vzczo6cGxhdGZv cm1Jbml0aWFsaXplV2ViUHIKICAgICBzZXRTaG91bGRQYWludE5hdGl2ZUNvbnRyb2xzKHBhcmFt ZXRlcnMuc2hvdWxkUGFpbnROYXRpdmVDb250cm9scyk7CiAKICNpZiBVU0UoQ0ZORVRXT1JLKQot I2lmIFVTRShDRlVSTFNUT1JBR0VTRVNTSU9OUykKLSAgICBpZiAoQ0ZVUkxTdG9yYWdlU2Vzc2lv blJlZiBkZWZhdWx0U3RvcmFnZVNlc3Npb24gPSB3a0Rlc2VyaWFsaXplU3RvcmFnZVNlc3Npb24o cGFyYW1ldGVycy5zZXJpYWxpemVkRGVmYXVsdFN0b3JhZ2VTZXNzaW9uLmdldCgpKSkgewotICAg ICAgICBSZXNvdXJjZUhhbmRsZTo6c2V0RGVmYXVsdFN0b3JhZ2VTZXNzaW9uKGRlZmF1bHRTdG9y YWdlU2Vzc2lvbik7CisgICAgQ0ZVUkxTdG9yYWdlU2Vzc2lvblJlZiBkZWZhdWx0U3RvcmFnZVNl c3Npb24gPSB3a0Rlc2VyaWFsaXplU3RvcmFnZVNlc3Npb24ocGFyYW1ldGVycy5zZXJpYWxpemVk RGVmYXVsdFN0b3JhZ2VTZXNzaW9uLmdldCgpKTsKKyAgICBSZXNvdXJjZUhhbmRsZTo6c2V0RGVm YXVsdFN0b3JhZ2VTZXNzaW9uKGRlZmF1bHRTdG9yYWdlU2Vzc2lvbik7CisKKyAgICBXZWJDb29r aWVNYW5hZ2VyOjpzaGFyZWQoKS5zZXRIVFRQQ29va2llQWNjZXB0UG9saWN5KHBhcmFtZXRlcnMu aW5pdGlhbEhUVFBDb29raWVBY2NlcHRQb2xpY3kpOworCisgICAgLy8gQnkgdXNpbmcgdGhlIGRl ZmF1bHQgc3RvcmFnZSBzZXNzaW9uIHRoYXQgY2FtZSBmcm9tIHRoZSBVSSBQcm9jZXNzLCB0aGUg V2ViIFByb2Nlc3MKKyAgICAvLyBhdXRvbWF0aWNhbGx5IHVzZSB0aGUgc2FtZSB0aGUgVVJMIENh Y2hlIGFzIFVJIFByb2Nlc3MuCisgICAgaWYgKGRlZmF1bHRTdG9yYWdlU2Vzc2lvbikKICAgICAg ICAgcmV0dXJuOwotICAgIH0KLSNlbmRpZiAvLyBVU0UoQ0ZVUkxTVE9SQUdFU0VTU0lPTlMpCiAK ICAgICBSZXRhaW5QdHI8Q0ZTdHJpbmdSZWY+IGNhY2hlUGF0aChBZG9wdENGLCBwYXJhbWV0ZXJz LmNmVVJMQ2FjaGVQYXRoLmNyZWF0ZUNGU3RyaW5nKCkpOwogICAgIGlmICghY2FjaGVQYXRoKQpA QCAtMTU0LDggKzE1Nyw2IEBAIHZvaWQgV2ViUHJvY2Vzczo6cGxhdGZvcm1Jbml0aWFsaXplV2Vi UHIKICAgICBSZXRhaW5QdHI8Q0ZVUkxDYWNoZVJlZj4gdWlQcm9jZXNzQ2FjaGUoQWRvcHRDRiwg Q0ZVUkxDYWNoZUNyZWF0ZShrQ0ZBbGxvY2F0b3JEZWZhdWx0LCBjYWNoZU1lbW9yeUNhcGFjaXR5 LCBjYWNoZURpc2tDYXBhY2l0eSwgY2FjaGVQYXRoLmdldCgpKSk7CiAgICAgQ0ZVUkxDYWNoZVNl dFNoYXJlZFVSTENhY2hlKHVpUHJvY2Vzc0NhY2hlLmdldCgpKTsKICNlbmRpZiAvLyBVU0UoQ0ZO RVRXT1JLKQotCi0gICAgV2ViQ29va2llTWFuYWdlcjo6c2hhcmVkKCkuc2V0SFRUUENvb2tpZUFj Y2VwdFBvbGljeShwYXJhbWV0ZXJzLmluaXRpYWxIVFRQQ29va2llQWNjZXB0UG9saWN5KTsKIH0K IAogdm9pZCBXZWJQcm9jZXNzOjpwbGF0Zm9ybVRlcm1pbmF0ZSgpCg==