77114 2012-01-26 10:15:49 -0800 IndexedDB: WebCore::IDBKey objects are leaked 2012-01-26 16:07:31 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jsbell jsbell dgrogan tony webkit.review.bot oldest_to_newest 543233 0 jsbell 2012-01-26 10:15:49 -0800 IndexedDB: WebCore::IDBKey objects are leaked 543234 1 124135 jsbell 2012-01-26 10:16:31 -0800 Created attachment 124135 Patch 543235 2 jsbell 2012-01-26 10:17:42 -0800 Tossing the patch up to let the bots have at it. On the one hand this looks like a no-brainer. On the other hand, it can't be this easy, can it? 543255 3 124135 dgrogan 2012-01-26 10:38:44 -0800 Comment on attachment 124135 Patch Looks promising. You'll have to get rid of the No new tests. (OOPS!) line. 543259 4 124135 dgrogan 2012-01-26 10:42:52 -0800 Comment on attachment 124135 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124135&action=review > Source/WebCore/storage/IDBKey.h:69 > + RefPtr<IDBKey> idbKey = adoptRef(new IDBKey()); Though, could you change these to use the common IDBKey::create() pattern? 543271 5 jsbell 2012-01-26 10:48:24 -0800 (In reply to comment #4) > (From update of attachment 124135 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124135&action=review > > > Source/WebCore/storage/IDBKey.h:69 > > + RefPtr<IDBKey> idbKey = adoptRef(new IDBKey()); > > Though, could you change these to use the common IDBKey::create() pattern? Do you mean, change these (where possible) to IDBKey::create(<Type>) with various type overloads, or adding a private static IDBKey::create() that the current createXXX() methods are changed to call? 543275 6 124135 dgrogan 2012-01-26 10:54:36 -0800 Comment on attachment 124135 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124135&action=review >>> Source/WebCore/storage/IDBKey.h:69 >>> + RefPtr<IDBKey> idbKey = adoptRef(new IDBKey()); >> >> Though, could you change these to use the common IDBKey::create() pattern? > > Do you mean, change these (where possible) to IDBKey::create(<Type>) with various type overloads, or adding a private static IDBKey::create() that the current createXXX() methods are changed to call? I had meant the second, but I hadn't noticed that these were already createXXX methods. So an IDBKey::create() probably doesn't add much. Request retracted. 543294 7 124149 jsbell 2012-01-26 11:04:48 -0800 Created attachment 124149 Patch 543298 8 jsbell 2012-01-26 11:07:52 -0800 tony@ - can you r? Again, this seems simple enough that I feel like I'm missing something. 543304 9 124149 tony 2012-01-26 11:12:24 -0800 Comment on attachment 124149 Patch This looks correct. There are other examples of this pattern in the code (e.g., SecurityOrigin::create). 543306 10 jsbell 2012-01-26 11:15:55 -0800 I'll verify this with valgrind if possible before landing it. 543569 11 jsbell 2012-01-26 15:29:41 -0800 Running under valgrind: sh tools/valgrind/chrome_tests.sh -t ui --gtest_filter='IndexedDBUILayoutTest.LayoutTests' * Without this patch it reports 11 suppressions of bug_109463 http://crbug.com/109463 * With this patch it reports 0 suppressions of bug_109463 I'm calling that good. 543617 12 124149 webkit.review.bot 2012-01-26 16:07:27 -0800 Comment on attachment 124149 Patch Clearing flags on attachment: 124149 Committed r106059: <http://trac.webkit.org/changeset/106059> 543618 13 webkit.review.bot 2012-01-26 16:07:31 -0800 All reviewed patches have been landed. Closing bug. 124135 2012-01-26 10:16:31 -0800 2012-01-26 11:04:45 -0800 Patch bug-77114-20120126101630.patch text/plain 2590 jsbell U3VidmVyc2lvbiBSZXZpc2lvbjogMTA1OTIwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYWE2YWJiMjc3OWYzZDQ4 YTEzNDRiOTYyMzQyODRkMTMxYmU5NDE1NC4uNDBjNWQ0OTBmODMxNmZhMDBhY2E4ZTUzM2M4ZmRk ZjNlOGNiYTcwNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTAxLTI2ICBKb3No dWEgQmVsbCAgPGpzYmVsbEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgSW5kZXhlZERCOiBXZWJD b3JlOjpJREJLZXkgb2JqZWN0cyBhcmUgbGVha2VkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NzExNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gKE9PUFMhKQorCisgICAgICAgICog c3RvcmFnZS9JREJLZXkuaDoKKyAgICAgICAgKFdlYkNvcmU6OklEQktleTo6Y3JlYXRlSW52YWxp ZCk6CisgICAgICAgIChXZWJDb3JlOjpJREJLZXk6OmNyZWF0ZU51bWJlcik6CisgICAgICAgIChX ZWJDb3JlOjpJREJLZXk6OmNyZWF0ZVN0cmluZyk6CisgICAgICAgIChXZWJDb3JlOjpJREJLZXk6 OmNyZWF0ZURhdGUpOgorICAgICAgICAoV2ViQ29yZTo6SURCS2V5OjpjcmVhdGVBcnJheSk6CisK IDIwMTItMDEtMjUgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgog CiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxMDU5MDYuCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9zdG9yYWdlL0lEQktleS5oIGIvU291cmNlL1dlYkNvcmUvc3RvcmFnZS9J REJLZXkuaAppbmRleCA0OGE2MTIzN2JhOGQ2NzcxYzg0NjVmNGE2YjY2MzI2MGU0N2M1MTgwLi4x N2M0ZWFlYTVlNDczMGMyMTlhZjkzYTQ4MTExZTU0N2Q4ZmQ3NjUxIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9zdG9yYWdlL0lEQktleS5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3N0b3JhZ2Uv SURCS2V5LmgKQEAgLTQxLDE0ICs0MSwxNCBAQCBwdWJsaWM6CiAKICAgICBzdGF0aWMgUGFzc1Jl ZlB0cjxJREJLZXk+IGNyZWF0ZUludmFsaWQoKQogICAgIHsKLSAgICAgICAgUmVmUHRyPElEQktl eT4gaWRiS2V5KG5ldyBJREJLZXkoKSk7CisgICAgICAgIFJlZlB0cjxJREJLZXk+IGlkYktleSA9 IGFkb3B0UmVmKG5ldyBJREJLZXkoKSk7CiAgICAgICAgIGlkYktleS0+bV90eXBlID0gSW52YWxp ZFR5cGU7CiAgICAgICAgIHJldHVybiBpZGJLZXkucmVsZWFzZSgpOwogICAgIH0KIAogICAgIHN0 YXRpYyBQYXNzUmVmUHRyPElEQktleT4gY3JlYXRlTnVtYmVyKGRvdWJsZSBudW1iZXIpCiAgICAg ewotICAgICAgICBSZWZQdHI8SURCS2V5PiBpZGJLZXkobmV3IElEQktleSgpKTsKKyAgICAgICAg UmVmUHRyPElEQktleT4gaWRiS2V5ID0gYWRvcHRSZWYobmV3IElEQktleSgpKTsKICAgICAgICAg aWRiS2V5LT5tX3R5cGUgPSBOdW1iZXJUeXBlOwogICAgICAgICBpZGJLZXktPm1fbnVtYmVyID0g bnVtYmVyOwogICAgICAgICBpZGJLZXktPm1fc2l6ZUVzdGltYXRlICs9IHNpemVvZihkb3VibGUp OwpAQCAtNTcsNyArNTcsNyBAQCBwdWJsaWM6CiAKICAgICBzdGF0aWMgUGFzc1JlZlB0cjxJREJL ZXk+IGNyZWF0ZVN0cmluZyhjb25zdCBTdHJpbmcmIHN0cmluZykKICAgICB7Ci0gICAgICAgIFJl ZlB0cjxJREJLZXk+IGlkYktleShuZXcgSURCS2V5KCkpOworICAgICAgICBSZWZQdHI8SURCS2V5 PiBpZGJLZXkgPSBhZG9wdFJlZihuZXcgSURCS2V5KCkpOwogICAgICAgICBpZGJLZXktPm1fdHlw ZSA9IFN0cmluZ1R5cGU7CiAgICAgICAgIGlkYktleS0+bV9zdHJpbmcgPSBzdHJpbmc7CiAgICAg ICAgIGlkYktleS0+bV9zaXplRXN0aW1hdGUgKz0gc3RyaW5nLmxlbmd0aCgpICogc2l6ZW9mKFVD aGFyKTsKQEAgLTY2LDcgKzY2LDcgQEAgcHVibGljOgogCiAgICAgc3RhdGljIFBhc3NSZWZQdHI8 SURCS2V5PiBjcmVhdGVEYXRlKGRvdWJsZSBkYXRlKQogICAgIHsKLSAgICAgICAgUmVmUHRyPElE QktleT4gaWRiS2V5KG5ldyBJREJLZXkoKSk7CisgICAgICAgIFJlZlB0cjxJREJLZXk+IGlkYktl eSA9IGFkb3B0UmVmKG5ldyBJREJLZXkoKSk7CiAgICAgICAgIGlkYktleS0+bV90eXBlID0gRGF0 ZVR5cGU7CiAgICAgICAgIGlkYktleS0+bV9kYXRlID0gZGF0ZTsKICAgICAgICAgaWRiS2V5LT5t X3NpemVFc3RpbWF0ZSArPSBzaXplb2YoZG91YmxlKTsKQEAgLTc1LDcgKzc1LDcgQEAgcHVibGlj OgogCiAgICAgc3RhdGljIFBhc3NSZWZQdHI8SURCS2V5PiBjcmVhdGVBcnJheShjb25zdCBLZXlB cnJheSYgYXJyYXkpCiAgICAgewotICAgICAgICBSZWZQdHI8SURCS2V5PiBpZGJLZXkobmV3IElE QktleSgpKTsKKyAgICAgICAgUmVmUHRyPElEQktleT4gaWRiS2V5ID0gYWRvcHRSZWYobmV3IElE QktleSgpKTsKICAgICAgICAgaWRiS2V5LT5tX3R5cGUgPSBBcnJheVR5cGU7CiAgICAgICAgIGlk YktleS0+bV9hcnJheSA9IGFycmF5OwogCg== 124149 2012-01-26 11:04:48 -0800 2012-01-26 16:07:27 -0800 Patch bug-77114-20120126110447.patch text/plain 2603 jsbell U3VidmVyc2lvbiBSZXZpc2lvbjogMTA1OTIwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYWE2YWJiMjc3OWYzZDQ4 YTEzNDRiOTYyMzQyODRkMTMxYmU5NDE1NC4uNmZlZGY1ZDdmMjFjMTkyOTFjOGViYzIwNjkyN2I1 YjNjNzhhMTQ2MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTAxLTI2ICBKb3No dWEgQmVsbCAgPGpzYmVsbEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgSW5kZXhlZERCOiBXZWJD b3JlOjpJREJLZXkgb2JqZWN0cyBhcmUgbGVha2VkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NzExNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEZpeGluZyBtZW1vcnkgbGVha3MsIG5vIG5ldyB0ZXN0cy4K KworICAgICAgICAqIHN0b3JhZ2UvSURCS2V5Lmg6CisgICAgICAgIChXZWJDb3JlOjpJREJLZXk6 OmNyZWF0ZUludmFsaWQpOgorICAgICAgICAoV2ViQ29yZTo6SURCS2V5OjpjcmVhdGVOdW1iZXIp OgorICAgICAgICAoV2ViQ29yZTo6SURCS2V5OjpjcmVhdGVTdHJpbmcpOgorICAgICAgICAoV2Vi Q29yZTo6SURCS2V5OjpjcmVhdGVEYXRlKToKKyAgICAgICAgKFdlYkNvcmU6OklEQktleTo6Y3Jl YXRlQXJyYXkpOgorCiAyMDEyLTAxLTI1ICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90 QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTA1OTA2Lgpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvc3RvcmFnZS9JREJLZXkuaCBiL1NvdXJjZS9XZWJD b3JlL3N0b3JhZ2UvSURCS2V5LmgKaW5kZXggNDhhNjEyMzdiYThkNjc3MWM4NDY1ZjRhNmI2NjMy NjBlNDdjNTE4MC4uMTdjNGVhZWE1ZTQ3MzBjMjE5YWY5M2E0ODExMWU1NDdkOGZkNzY1MSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvc3RvcmFnZS9JREJLZXkuaAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9zdG9yYWdlL0lEQktleS5oCkBAIC00MSwxNCArNDEsMTQgQEAgcHVibGljOgogCiAgICAg c3RhdGljIFBhc3NSZWZQdHI8SURCS2V5PiBjcmVhdGVJbnZhbGlkKCkKICAgICB7Ci0gICAgICAg IFJlZlB0cjxJREJLZXk+IGlkYktleShuZXcgSURCS2V5KCkpOworICAgICAgICBSZWZQdHI8SURC S2V5PiBpZGJLZXkgPSBhZG9wdFJlZihuZXcgSURCS2V5KCkpOwogICAgICAgICBpZGJLZXktPm1f dHlwZSA9IEludmFsaWRUeXBlOwogICAgICAgICByZXR1cm4gaWRiS2V5LnJlbGVhc2UoKTsKICAg ICB9CiAKICAgICBzdGF0aWMgUGFzc1JlZlB0cjxJREJLZXk+IGNyZWF0ZU51bWJlcihkb3VibGUg bnVtYmVyKQogICAgIHsKLSAgICAgICAgUmVmUHRyPElEQktleT4gaWRiS2V5KG5ldyBJREJLZXko KSk7CisgICAgICAgIFJlZlB0cjxJREJLZXk+IGlkYktleSA9IGFkb3B0UmVmKG5ldyBJREJLZXko KSk7CiAgICAgICAgIGlkYktleS0+bV90eXBlID0gTnVtYmVyVHlwZTsKICAgICAgICAgaWRiS2V5 LT5tX251bWJlciA9IG51bWJlcjsKICAgICAgICAgaWRiS2V5LT5tX3NpemVFc3RpbWF0ZSArPSBz aXplb2YoZG91YmxlKTsKQEAgLTU3LDcgKzU3LDcgQEAgcHVibGljOgogCiAgICAgc3RhdGljIFBh c3NSZWZQdHI8SURCS2V5PiBjcmVhdGVTdHJpbmcoY29uc3QgU3RyaW5nJiBzdHJpbmcpCiAgICAg ewotICAgICAgICBSZWZQdHI8SURCS2V5PiBpZGJLZXkobmV3IElEQktleSgpKTsKKyAgICAgICAg UmVmUHRyPElEQktleT4gaWRiS2V5ID0gYWRvcHRSZWYobmV3IElEQktleSgpKTsKICAgICAgICAg aWRiS2V5LT5tX3R5cGUgPSBTdHJpbmdUeXBlOwogICAgICAgICBpZGJLZXktPm1fc3RyaW5nID0g c3RyaW5nOwogICAgICAgICBpZGJLZXktPm1fc2l6ZUVzdGltYXRlICs9IHN0cmluZy5sZW5ndGgo KSAqIHNpemVvZihVQ2hhcik7CkBAIC02Niw3ICs2Niw3IEBAIHB1YmxpYzoKIAogICAgIHN0YXRp YyBQYXNzUmVmUHRyPElEQktleT4gY3JlYXRlRGF0ZShkb3VibGUgZGF0ZSkKICAgICB7Ci0gICAg ICAgIFJlZlB0cjxJREJLZXk+IGlkYktleShuZXcgSURCS2V5KCkpOworICAgICAgICBSZWZQdHI8 SURCS2V5PiBpZGJLZXkgPSBhZG9wdFJlZihuZXcgSURCS2V5KCkpOwogICAgICAgICBpZGJLZXkt Pm1fdHlwZSA9IERhdGVUeXBlOwogICAgICAgICBpZGJLZXktPm1fZGF0ZSA9IGRhdGU7CiAgICAg ICAgIGlkYktleS0+bV9zaXplRXN0aW1hdGUgKz0gc2l6ZW9mKGRvdWJsZSk7CkBAIC03NSw3ICs3 NSw3IEBAIHB1YmxpYzoKIAogICAgIHN0YXRpYyBQYXNzUmVmUHRyPElEQktleT4gY3JlYXRlQXJy YXkoY29uc3QgS2V5QXJyYXkmIGFycmF5KQogICAgIHsKLSAgICAgICAgUmVmUHRyPElEQktleT4g aWRiS2V5KG5ldyBJREJLZXkoKSk7CisgICAgICAgIFJlZlB0cjxJREJLZXk+IGlkYktleSA9IGFk b3B0UmVmKG5ldyBJREJLZXkoKSk7CiAgICAgICAgIGlkYktleS0+bV90eXBlID0gQXJyYXlUeXBl OwogICAgICAgICBpZGJLZXktPm1fYXJyYXkgPSBhcnJheTsKIAo=