86654 2012-05-16 10:50:14 -0700 [chromium] Clear the m_private pointer when destroying WebFilterOperations to avoid assert in WebPrivateOwnPtr 2012-05-16 16:34:36 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 danakj danakj abarth cc-bugs dglazkov enne fishd jamesr tkent+wkapi webkit.review.bot oldest_to_newest 625627 0 danakj 2012-05-16 10:50:14 -0700 [chromium] Clear the m_private pointer when destroying WebFilterOperations to avoid assert in WebPrivateOwnPtr 625631 1 142298 danakj 2012-05-16 10:51:29 -0700 Created attachment 142298 Patch The ::reset() method is private on other WebClasses, but I can't imagine why it would be. So I've made it private here. If this is crazy please say so! 625635 2 webkit.review.bot 2012-05-16 10:55:36 -0700 Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. 625739 3 enne 2012-05-16 12:30:51 -0700 It seems like it should be public to me. What classes are you seeing that on? 625749 4 142298 jamesr 2012-05-16 12:35:21 -0700 Comment on attachment 142298 Patch It's normally public since it makes sense for someone using a WebFilterOperations to want to get a fresh one without having to make a new object (or wrap it in a pointer or the like). 625785 5 142330 danakj 2012-05-16 13:20:19 -0700 Created attachment 142330 Patch K made reset() public. Dropping clear() since it's redundant then. Adding private destroy() to clear the m_private pointer so that toFilterOperations() can remain const. 625924 6 142330 jamesr 2012-05-16 15:24:13 -0700 Comment on attachment 142330 Patch reset() in WebKit API normally puts you in a null state (and there's often a bool isNull() call to check it), so it's a bit odd to have initialize() construct something non-null. Maybe leave initialize() and have ~ call reset() ? 625941 7 danakj 2012-05-16 15:42:19 -0700 I tried this at first, but the problem is toFilterOperations() is const and returns a reference, not a pointer. So if the m_private is null, then you can't call toFilterOperations() without a segfault, because you can't change m_private. Options: 1) Changing toFilterOperations() to non-const: breaks the WebLayer API which takes a const WebFilterOperations&. 2) Changing toFilterOperations() to return a pointer? 3) Make a static empty FilterOperations object to return when m_private is NULL? 4) Don't let m_private be NULL. 625944 8 jamesr 2012-05-16 15:44:18 -0700 Ah, I see. Having m_private always be non-NULL seems like the best of those options. 625945 9 142330 jamesr 2012-05-16 15:44:42 -0700 Comment on attachment 142330 Patch R=me then. Thanks for the explanation. 625949 10 142330 danakj 2012-05-16 15:46:04 -0700 Comment on attachment 142330 Patch Thanks! 625997 11 142330 webkit.review.bot 2012-05-16 16:34:31 -0700 Comment on attachment 142330 Patch Clearing flags on attachment: 142330 Committed r117362: <http://trac.webkit.org/changeset/117362> 625998 12 webkit.review.bot 2012-05-16 16:34:36 -0700 All reviewed patches have been landed. Closing bug. 142298 2012-05-16 10:51:29 -0700 2012-05-16 13:20:14 -0700 Patch bug-86654-20120516135128.patch text/plain 3025 danakj U3VidmVyc2lvbiBSZXZpc2lvbjogMTE3MDc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvUGxhdGZvcm0v Q2hhbmdlTG9nIGIvU291cmNlL1BsYXRmb3JtL0NoYW5nZUxvZwppbmRleCBkNGZmZmYyNTFlOTQx MmZkZjMyMjhkNTA3OWJjY2RhOTc2NjQ1ZTNmLi5kN2EzNWE5NjM3MTViYzk1OTIxMWM4ZjNhM2Jm NWUwYzJjMmEzYjc4IDEwMDY0NAotLS0gYS9Tb3VyY2UvUGxhdGZvcm0vQ2hhbmdlTG9nCisrKyBi L1NvdXJjZS9QbGF0Zm9ybS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMi0wNS0xNiAg RGFuYSBKYW5zZW5zICA8ZGFuYWtqQGNocm9taXVtLm9yZz4KKworICAgICAgICBbY2hyb21pdW1d IENsZWFyIHRoZSBtX3ByaXZhdGUgcG9pbnRlciB3aGVuIGRlc3Ryb3lpbmcgV2ViRmlsdGVyT3Bl cmF0aW9ucyB0byBhdm9pZCBhc3NlcnQgaW4gV2ViUHJpdmF0ZU93blB0cgorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODY2NTQKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGNocm9taXVtL3B1YmxpYy9XZWJG aWx0ZXJPcGVyYXRpb25zLmg6CisgICAgICAgIChXZWJLaXQ6OldlYkZpbHRlck9wZXJhdGlvbnM6 On5XZWJGaWx0ZXJPcGVyYXRpb25zKToKKyAgICAgICAgKFdlYkZpbHRlck9wZXJhdGlvbnMpOgor CiAyMDEyLTA1LTE0ICBHYXZpbiBQZXRlcnMgIDxnYXZpbnBAY2hyb21pdW0ub3JnPgogCiAgICAg ICAgIEFkZCBQcmVyZW5kZXJlciwgUHJlcmVuZGVySGFuZGxlIGFuZCBhIGNocm9taXVtIGludGVy ZmFjZSBmb3IgUHJlcmVuZGVyaW5nLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IDdiYmM3MjkwMmY3NjMzYzhiNzlj YmQzMWZkYTA5OTNkN2UxZmE4ZWIuLjRiYzdkNTUyZDlkODgyNzViOTE0Nzg2YmYxMTBhMzVlOGNk YjA2NzQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2Uv V2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMi0wNS0xNiAgRGFuYSBKYW5z ZW5zICA8ZGFuYWtqQGNocm9taXVtLm9yZz4KKworICAgICAgICBbY2hyb21pdW1dIENsZWFyIHRo ZSBtX3ByaXZhdGUgcG9pbnRlciB3aGVuIGRlc3Ryb3lpbmcgV2ViRmlsdGVyT3BlcmF0aW9ucyB0 byBhdm9pZCBhc3NlcnQgaW4gV2ViUHJpdmF0ZU93blB0cgorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODY2NTQKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHBsYXRmb3JtL2Nocm9taXVtL3N1cHBvcnQvV2Vi RmlsdGVyT3BlcmF0aW9ucy5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYkZpbHRlck9wZXJhdGlv bnM6OnJlc2V0KToKKyAgICAgICAgKFdlYktpdCk6CisKIDIwMTItMDUtMTUgIEpvY2VseW4gVHVy Y290dGUgIDxqb2NlbHluLnR1cmNvdHRlQG5va2lhLmNvbT4KIAogICAgICAgICBbUXRdIGh0dHAv dGVzdHMveG1saHR0cHJlcXVlc3QveG1saHR0cHJlcXVlc3QtY2hlY2staGVhZC1yZWFkeXN0YXRl LWZvci00MDQuaHRtbCB0aW1lcyBvdXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9QbGF0Zm9ybS9jaHJv bWl1bS9wdWJsaWMvV2ViRmlsdGVyT3BlcmF0aW9ucy5oIGIvU291cmNlL1BsYXRmb3JtL2Nocm9t aXVtL3B1YmxpYy9XZWJGaWx0ZXJPcGVyYXRpb25zLmgKaW5kZXggNzU3NDljNmNmMjdlNzI3ODQz ZDExMjk2ZDVlOGRhOTc0NzZiY2NiYi4uMGRjNjUzNTNiODU3ODgzZWMwNTg0NDM5N2FmODEzYzUw MzA4NGIzYSAxMDA2NDQKLS0tIGEvU291cmNlL1BsYXRmb3JtL2Nocm9taXVtL3B1YmxpYy9XZWJG aWx0ZXJPcGVyYXRpb25zLmgKKysrIGIvU291cmNlL1BsYXRmb3JtL2Nocm9taXVtL3B1YmxpYy9X ZWJGaWx0ZXJPcGVyYXRpb25zLmgKQEAgLTQwLDYgKzQwLDcgQEAgc3RydWN0IFdlYkZpbHRlck9w ZXJhdGlvbjsKIGNsYXNzIFdlYkZpbHRlck9wZXJhdGlvbnMgewogcHVibGljOgogICAgIFdlYkZp bHRlck9wZXJhdGlvbnMoKSB7IGluaXRpYWxpemUoKTsgfQorICAgIH5XZWJGaWx0ZXJPcGVyYXRp b25zKCkgeyByZXNldCgpOyB9CiAKICAgICBXRUJLSVRfRVhQT1JUIHZvaWQgYXBwZW5kKGNvbnN0 IFdlYkZpbHRlck9wZXJhdGlvbiYpOwogCkBAIC01Miw2ICs1Myw3IEBAIHB1YmxpYzoKIAogcHJp dmF0ZToKICAgICBXRUJLSVRfRVhQT1JUIHZvaWQgaW5pdGlhbGl6ZSgpOworICAgIFdFQktJVF9F WFBPUlQgdm9pZCByZXNldCgpOwogCiAgICAgV2ViUHJpdmF0ZU93blB0cjxXZWJDb3JlOjpGaWx0 ZXJPcGVyYXRpb25zPiBtX3ByaXZhdGU7CiB9OwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vY2hyb21pdW0vc3VwcG9ydC9XZWJGaWx0ZXJPcGVyYXRpb25zLmNwcCBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL3N1cHBvcnQvV2ViRmlsdGVyT3BlcmF0aW9ucy5j cHAKaW5kZXggZDYwZDkyODdhNmI1OTkxNDEzMmUwMGI0NDIyMjE1N2E2OTQwNTVmZC4uZjQyYzRh YjE2NWY4M2YwODIyZDRmNzAxOTgzNGQwNDNjNDVjMjJlZCAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vc3VwcG9ydC9XZWJGaWx0ZXJPcGVyYXRpb25zLmNwcAor KysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9zdXBwb3J0L1dlYkZpbHRlck9w ZXJhdGlvbnMuY3BwCkBAIC00MCw2ICs0MCwxMSBAQCB2b2lkIFdlYkZpbHRlck9wZXJhdGlvbnM6 OmluaXRpYWxpemUoKQogICAgIG1fcHJpdmF0ZS5yZXNldChuZXcgRmlsdGVyT3BlcmF0aW9ucygp KTsKIH0KIAordm9pZCBXZWJGaWx0ZXJPcGVyYXRpb25zOjpyZXNldCgpCit7CisgICAgbV9wcml2 YXRlLnJlc2V0KDApOworfQorCiB2b2lkIFdlYkZpbHRlck9wZXJhdGlvbnM6OmFwcGVuZChjb25z dCBXZWJGaWx0ZXJPcGVyYXRpb24mIGZpbHRlcikKIHsKICAgICBzd2l0Y2ggKGZpbHRlci50eXBl KSB7Cg== 142330 2012-05-16 13:20:19 -0700 2012-05-16 16:34:31 -0700 Patch bug-86654-20120516162018.patch text/plain 3719 danakj U3VidmVyc2lvbiBSZXZpc2lvbjogMTE3MDc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvUGxhdGZvcm0v Q2hhbmdlTG9nIGIvU291cmNlL1BsYXRmb3JtL0NoYW5nZUxvZwppbmRleCBkNGZmZmYyNTFlOTQx MmZkZjMyMjhkNTA3OWJjY2RhOTc2NjQ1ZTNmLi5kN2EzNWE5NjM3MTViYzk1OTIxMWM4ZjNhM2Jm NWUwYzJjMmEzYjc4IDEwMDY0NAotLS0gYS9Tb3VyY2UvUGxhdGZvcm0vQ2hhbmdlTG9nCisrKyBi L1NvdXJjZS9QbGF0Zm9ybS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMi0wNS0xNiAg RGFuYSBKYW5zZW5zICA8ZGFuYWtqQGNocm9taXVtLm9yZz4KKworICAgICAgICBbY2hyb21pdW1d IENsZWFyIHRoZSBtX3ByaXZhdGUgcG9pbnRlciB3aGVuIGRlc3Ryb3lpbmcgV2ViRmlsdGVyT3Bl cmF0aW9ucyB0byBhdm9pZCBhc3NlcnQgaW4gV2ViUHJpdmF0ZU93blB0cgorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODY2NTQKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGNocm9taXVtL3B1YmxpYy9XZWJG aWx0ZXJPcGVyYXRpb25zLmg6CisgICAgICAgIChXZWJLaXQ6OldlYkZpbHRlck9wZXJhdGlvbnM6 On5XZWJGaWx0ZXJPcGVyYXRpb25zKToKKyAgICAgICAgKFdlYkZpbHRlck9wZXJhdGlvbnMpOgor CiAyMDEyLTA1LTE0ICBHYXZpbiBQZXRlcnMgIDxnYXZpbnBAY2hyb21pdW0ub3JnPgogCiAgICAg ICAgIEFkZCBQcmVyZW5kZXJlciwgUHJlcmVuZGVySGFuZGxlIGFuZCBhIGNocm9taXVtIGludGVy ZmFjZSBmb3IgUHJlcmVuZGVyaW5nLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IDdiYmM3MjkwMmY3NjMzYzhiNzlj YmQzMWZkYTA5OTNkN2UxZmE4ZWIuLmYwY2E3ZTA0NWM3NjQ5ZGVhMGRlYWY5YTk1ZDRiMmJiYzgy MDI2MjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2Uv V2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAxMi0wNS0xNiAgRGFuYSBKYW5z ZW5zICA8ZGFuYWtqQGNocm9taXVtLm9yZz4KKworICAgICAgICBbY2hyb21pdW1dIENsZWFyIHRo ZSBtX3ByaXZhdGUgcG9pbnRlciB3aGVuIGRlc3Ryb3lpbmcgV2ViRmlsdGVyT3BlcmF0aW9ucyB0 byBhdm9pZCBhc3NlcnQgaW4gV2ViUHJpdmF0ZU93blB0cgorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODY2NTQKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHBsYXRmb3JtL2Nocm9taXVtL3N1cHBvcnQvV2Vi RmlsdGVyT3BlcmF0aW9ucy5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYkZpbHRlck9wZXJhdGlv bnM6OnJlc2V0KToKKyAgICAgICAgKFdlYktpdDo6V2ViRmlsdGVyT3BlcmF0aW9uczo6ZGVzdHJv eSk6CisgICAgICAgIChXZWJLaXQpOgorCiAyMDEyLTA1LTE1ICBKb2NlbHluIFR1cmNvdHRlICA8 am9jZWx5bi50dXJjb3R0ZUBub2tpYS5jb20+CiAKICAgICAgICAgW1F0XSBodHRwL3Rlc3RzL3ht bGh0dHByZXF1ZXN0L3htbGh0dHByZXF1ZXN0LWNoZWNrLWhlYWQtcmVhZHlzdGF0ZS1mb3ItNDA0 Lmh0bWwgdGltZXMgb3V0CmRpZmYgLS1naXQgYS9Tb3VyY2UvUGxhdGZvcm0vY2hyb21pdW0vcHVi bGljL1dlYkZpbHRlck9wZXJhdGlvbnMuaCBiL1NvdXJjZS9QbGF0Zm9ybS9jaHJvbWl1bS9wdWJs aWMvV2ViRmlsdGVyT3BlcmF0aW9ucy5oCmluZGV4IDc1NzQ5YzZjZjI3ZTcyNzg0M2QxMTI5NmQ1 ZThkYTk3NDc2YmNjYmIuLjU2ODA1ZDAwM2JlYWRkMzg1MjVlODlmY2UxYjgyZTQ5ZWMwNTBjODMg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9QbGF0Zm9ybS9jaHJvbWl1bS9wdWJsaWMvV2ViRmlsdGVyT3Bl cmF0aW9ucy5oCisrKyBiL1NvdXJjZS9QbGF0Zm9ybS9jaHJvbWl1bS9wdWJsaWMvV2ViRmlsdGVy T3BlcmF0aW9ucy5oCkBAIC0zOSwxOSArMzksMTggQEAgc3RydWN0IFdlYkZpbHRlck9wZXJhdGlv bjsKIC8vIEFuIG9yZGVyZWQgc2V0IG9mIGZpbHRlciBvcGVyYXRpb25zLgogY2xhc3MgV2ViRmls dGVyT3BlcmF0aW9ucyB7CiBwdWJsaWM6Ci0gICAgV2ViRmlsdGVyT3BlcmF0aW9ucygpIHsgaW5p dGlhbGl6ZSgpOyB9CisgICAgV2ViRmlsdGVyT3BlcmF0aW9ucygpIHsgcmVzZXQoKTsgfQorICAg IH5XZWJGaWx0ZXJPcGVyYXRpb25zKCkgeyBkZXN0cm95KCk7IH0KIAorICAgIFdFQktJVF9FWFBP UlQgdm9pZCByZXNldCgpOwogICAgIFdFQktJVF9FWFBPUlQgdm9pZCBhcHBlbmQoY29uc3QgV2Vi RmlsdGVyT3BlcmF0aW9uJik7CiAKLSAgICAvLyBSZW1vdmVzIGFsbCBmaWx0ZXIgb3BlcmF0aW9u cy4KLSAgICBXRUJLSVRfRVhQT1JUIHZvaWQgY2xlYXIoKTsKLQogI2lmIFdFQktJVF9JTVBMRU1F TlRBVElPTgogICAgIGNvbnN0IFdlYkNvcmU6OkZpbHRlck9wZXJhdGlvbnMmIHRvRmlsdGVyT3Bl cmF0aW9ucygpIGNvbnN0OwogI2VuZGlmCiAKIHByaXZhdGU6Ci0gICAgV0VCS0lUX0VYUE9SVCB2 b2lkIGluaXRpYWxpemUoKTsKKyAgICBXRUJLSVRfRVhQT1JUIHZvaWQgZGVzdHJveSgpOwogCiAg ICAgV2ViUHJpdmF0ZU93blB0cjxXZWJDb3JlOjpGaWx0ZXJPcGVyYXRpb25zPiBtX3ByaXZhdGU7 CiB9OwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vc3VwcG9y dC9XZWJGaWx0ZXJPcGVyYXRpb25zLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9t aXVtL3N1cHBvcnQvV2ViRmlsdGVyT3BlcmF0aW9ucy5jcHAKaW5kZXggZDYwZDkyODdhNmI1OTkx NDEzMmUwMGI0NDIyMjE1N2E2OTQwNTVmZC4uMWU1Yzg1ZjE5MmUwNzA5YmUzNmU1NTU4YzQxNWNj NDAzNzIxMmQwNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0v c3VwcG9ydC9XZWJGaWx0ZXJPcGVyYXRpb25zLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9jaHJvbWl1bS9zdXBwb3J0L1dlYkZpbHRlck9wZXJhdGlvbnMuY3BwCkBAIC0zNSwxMSAr MzUsMTYgQEAgdXNpbmcgbmFtZXNwYWNlIFdlYkNvcmU7CiAKIG5hbWVzcGFjZSBXZWJLaXQgewog Ci12b2lkIFdlYkZpbHRlck9wZXJhdGlvbnM6OmluaXRpYWxpemUoKQordm9pZCBXZWJGaWx0ZXJP cGVyYXRpb25zOjpyZXNldCgpCiB7CiAgICAgbV9wcml2YXRlLnJlc2V0KG5ldyBGaWx0ZXJPcGVy YXRpb25zKCkpOwogfQogCit2b2lkIFdlYkZpbHRlck9wZXJhdGlvbnM6OmRlc3Ryb3koKQorewor ICAgIG1fcHJpdmF0ZS5yZXNldCgwKTsKK30KKwogdm9pZCBXZWJGaWx0ZXJPcGVyYXRpb25zOjph cHBlbmQoY29uc3QgV2ViRmlsdGVyT3BlcmF0aW9uJiBmaWx0ZXIpCiB7CiAgICAgc3dpdGNoIChm aWx0ZXIudHlwZSkgewpAQCAtNjYsMTEgKzcxLDYgQEAgdm9pZCBXZWJGaWx0ZXJPcGVyYXRpb25z OjphcHBlbmQoY29uc3QgV2ViRmlsdGVyT3BlcmF0aW9uJiBmaWx0ZXIpCiAgICAgfQogfQogCi12 b2lkIFdlYkZpbHRlck9wZXJhdGlvbnM6OmNsZWFyKCkKLXsKLSAgICBtX3ByaXZhdGUtPm9wZXJh dGlvbnMoKS5jbGVhcigpOwotfQotCiBjb25zdCBGaWx0ZXJPcGVyYXRpb25zJiBXZWJGaWx0ZXJP cGVyYXRpb25zOjp0b0ZpbHRlck9wZXJhdGlvbnMoKSBjb25zdAogewogICAgIHJldHVybiAqbV9w cml2YXRlLmdldCgpOwo=