112224 2013-03-12 20:20:06 -0700 IndexedDB: Record attempts to open a path with non-ascii characters 2013-03-13 15:01:42 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 dgrogan dgrogan alecflett jsbell mkwst tony webkit.review.bot oldest_to_newest 854022 0 dgrogan 2013-03-12 20:20:06 -0700 IndexedDB: Record when we try to open a path with non-ascii characters 854023 1 192861 dgrogan 2013-03-12 20:21:33 -0700 Created attachment 192861 Patch 854024 2 192862 dgrogan 2013-03-12 20:22:46 -0700 Created attachment 192862 Patch 854025 3 dgrogan 2013-03-12 20:26:13 -0700 Josh, could you take a look at this? 854049 4 192862 jsbell 2013-03-12 21:45:30 -0700 Comment on attachment 192862 Patch Looks fine. We should also do local testing on Windows to rule out anything obvious. 854202 5 192862 mkwst 2013-03-13 04:56:59 -0700 Comment on attachment 192862 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192862&action=review Quick drive-by comment. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:396 > + HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenAttemptNonASCII, IDBLevelDBBackingStoreOpenMax); It looks like you're calling `HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", ???, IDBLevelDBBackingStoreOpenMax);` ~10 times in this file. It might be worth creating a static helper function for clarity, similar to what you've already done with `recordInternalError`. 854203 6 mkwst 2013-03-13 04:57:15 -0700 (In reply to comment #5) > (From update of attachment 192862 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192862&action=review > > Quick drive-by comment. > > > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:396 > > + HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenAttemptNonASCII, IDBLevelDBBackingStoreOpenMax); > > It looks like you're calling `HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", ???, IDBLevelDBBackingStoreOpenMax);` ~10 times in this file. It might be worth creating a static helper function for clarity, similar to what you've already done with `recordInternalError`. (in a future patch, of course... :) ) 854455 7 dgrogan 2013-03-13 12:08:28 -0700 (In reply to comment #4) > (From update of attachment 192862 [details]) > Looks fine. > We should also do local testing on Windows to rule out anything obvious. I did my best with rdesktop, http://download-chromium.appspot.com/ and --enable-dcheck. cursor-advance.html ran fine, I shutdown and reopened, cursor-advance ran fine again. This isn't a perfect test because, among other reasons, cursor-advance starts with a a deleteDatabase. Though deleteDatabase goes through the IDBBackingStore::open path, so it probably would have caught anything obvious. 854456 8 192862 dgrogan 2013-03-13 12:09:04 -0700 Comment on attachment 192862 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192862&action=review >>> Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:396 >>> + HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenAttemptNonASCII, IDBLevelDBBackingStoreOpenMax); >> >> It looks like you're calling `HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", ???, IDBLevelDBBackingStoreOpenMax);` ~10 times in this file. It might be worth creating a static helper function for clarity, similar to what you've already done with `recordInternalError`. > > (in a future patch, of course... :) ) You're right, readers would have an easier time realizing that it all goes to the same place. 854459 9 dgrogan 2013-03-13 12:12:46 -0700 Tony, could you review this? 854690 10 192862 webkit.review.bot 2013-03-13 15:01:38 -0700 Comment on attachment 192862 Patch Clearing flags on attachment: 192862 Committed r145760: <http://trac.webkit.org/changeset/145760> 854691 11 webkit.review.bot 2013-03-13 15:01:42 -0700 All reviewed patches have been landed. Closing bug. 192861 2013-03-12 20:21:33 -0700 2013-03-12 20:22:45 -0700 Patch bug-112224-20130312201731.patch text/plain 2284 dgrogan U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1NjUzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTQ0MDY0YmJiZmNlMzAx YzJjZGI3OGEzMzBkOWI3ZDBiMjllOTVjYi4uMmYwNDM5ZjJmYTMzODI0YzAwNzMyYWMzYjFmNjg0 OTQxMjU3Yjg5NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAzLTEyICBEYXZp ZCBHcm9nYW4gIDxkZ3JvZ2FuQGNocm9taXVtLm9yZz4KKworICAgICAgICBJbmRleGVkREI6IFJl Y29yZCBhdHRlbXBzIHRvIG9wZW4gYSBwYXRoIHdpdGggbm9uLWFzY2lpIGNoYXJhY3RlcnMKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMjIyNAorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0 cywgdGhlcmUncyBubyB3YXkgdG8gdGVzdCBoaXN0b2dyYW1taW5nIHRoYXQgSSBrbm93IG9mLiBJ CisgICAgICAgIHJhbiBjaHJvbWUgbG9jYWxseSB3aXRoIGEgbm9uIGFzY2lpIHBhdGggdGhvdWdo LgorCisgICAgICAgICogTW9kdWxlcy9pbmRleGVkZGIvSURCQmFja2luZ1N0b3JlLmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OklEQkJhY2tpbmdTdG9yZTo6b3Blbik6CisKIDIwMTMtMDMtMTIgIFNo ZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAgIFVucmV2 aWV3ZWQsIHJvbGxpbmcgb3V0IHIxNDUyODAuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9N b2R1bGVzL2luZGV4ZWRkYi9JREJCYWNraW5nU3RvcmUuY3BwIGIvU291cmNlL1dlYkNvcmUvTW9k dWxlcy9pbmRleGVkZGIvSURCQmFja2luZ1N0b3JlLmNwcAppbmRleCAwYWJjZmVkODZjNzRhNzE1 ZmY2NjU2MzBlODIzZWY3ZDRiNzc0ZWQ0Li4xZjk5ZjFhMjk5ZTkxOTE4NWUyYWNhNzA5N2QyYzAz MzEzODlhODY4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL2luZGV4ZWRkYi9J REJCYWNraW5nU3RvcmUuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvaW5kZXhlZGRi L0lEQkJhY2tpbmdTdG9yZS5jcHAKQEAgLTM2NSw2ICszNjUsNyBAQCBlbnVtIElEQkxldmVsREJC YWNraW5nU3RvcmVPcGVuUmVzdWx0IHsKICAgICBJREJMZXZlbERCQmFja2luZ1N0b3JlT3BlbkZh aWxlZElPRXJyQ2hlY2tpbmdTY2hlbWEsCiAgICAgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5G YWlsZWRVbmtub3duRXJyLAogICAgIElEQkxldmVsREJCYWNraW5nU3RvcmVPcGVuTWVtb3J5RmFp bGVkLAorICAgIElEQkxldmVsREJCYWNraW5nU3RvcmVPcGVuQXR0ZW1wdE5vbkFTQ0lJLAogICAg IElEQkxldmVsREJCYWNraW5nU3RvcmVPcGVuTWF4LAogfTsKIApAQCAtMzkxLDYgKzM5Miw4IEBA IFBhc3NSZWZQdHI8SURCQmFja2luZ1N0b3JlPiBJREJCYWNraW5nU3RvcmU6Om9wZW4oU2VjdXJp dHlPcmlnaW4qIHNlY3VyaXR5T3JpZ2luCiAgICAgICAgIH0KICAgICAgICAgSGlzdG9ncmFtU3Vw cG9ydDo6aGlzdG9ncmFtRW51bWVyYXRpb24oIldlYkNvcmUuSW5kZXhlZERCLkJhY2tpbmdTdG9y ZS5PcGVuU3RhdHVzIiwgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5NZW1vcnlTdWNjZXNzLCBJ REJMZXZlbERCQmFja2luZ1N0b3JlT3Blbk1heCk7CiAgICAgfSBlbHNlIHsKKyAgICAgICAgaWYg KCFwYXRoQmFzZS5jb250YWluc09ubHlBU0NJSSgpKQorICAgICAgICAgICAgSGlzdG9ncmFtU3Vw cG9ydDo6aGlzdG9ncmFtRW51bWVyYXRpb24oIldlYkNvcmUuSW5kZXhlZERCLkJhY2tpbmdTdG9y ZS5PcGVuU3RhdHVzIiwgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5BdHRlbXB0Tm9uQVNDSUks IElEQkxldmVsREJCYWNraW5nU3RvcmVPcGVuTWF4KTsKICAgICAgICAgaWYgKCFtYWtlQWxsRGly ZWN0b3JpZXMocGF0aEJhc2UpKSB7CiAgICAgICAgICAgICBMT0dfRVJST1IoIlVuYWJsZSB0byBj cmVhdGUgSW5kZXhlZERCIGRhdGFiYXNlIHBhdGggJXMiLCBwYXRoQmFzZS51dGY4KCkuZGF0YSgp KTsKICAgICAgICAgICAgIEhpc3RvZ3JhbVN1cHBvcnQ6Omhpc3RvZ3JhbUVudW1lcmF0aW9uKCJX ZWJDb3JlLkluZGV4ZWREQi5CYWNraW5nU3RvcmUuT3BlblN0YXR1cyIsIElEQkxldmVsREJCYWNr aW5nU3RvcmVPcGVuRmFpbGVkRGlyZWN0b3J5LCBJREJMZXZlbERCQmFja2luZ1N0b3JlT3Blbk1h eCk7Cg== 192862 2013-03-12 20:22:46 -0700 2013-03-13 15:01:38 -0700 Patch bug-112224-20130312201844.patch text/plain 2352 dgrogan U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1NjUzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTQ0MDY0YmJiZmNlMzAx YzJjZGI3OGEzMzBkOWI3ZDBiMjllOTVjYi4uYWExMmE2ZTNjN2M3YTI4ZWM4ODk2ODQ2ZGU5MzI3 MDhhY2MzMjAyMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTAzLTEyICBEYXZp ZCBHcm9nYW4gIDxkZ3JvZ2FuQGNocm9taXVtLm9yZz4KKworICAgICAgICBJbmRleGVkREI6IFJl Y29yZCBhdHRlbXBzIHRvIG9wZW4gYSBwYXRoIHdpdGggbm9uLWFzY2lpIGNoYXJhY3RlcnMKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMjIyNAorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdlIHdhbnQgdG8g c2VlIGlmIHRoYXQncyBjb3JyZWxhdGVkIHdpdGggZXJyb3JzIG9uIG9wZW4uCisKKyAgICAgICAg Tm8gbmV3IHRlc3RzLCB0aGVyZSdzIG5vIHdheSB0byB0ZXN0IGhpc3RvZ3JhbW1pbmcgdGhhdCBJ IGtub3cgb2YuIEkKKyAgICAgICAgcmFuIGNocm9tZSBsb2NhbGx5IHdpdGggYSBub24gYXNjaWkg cGF0aCB0aG91Z2guCisKKyAgICAgICAgKiBNb2R1bGVzL2luZGV4ZWRkYi9JREJCYWNraW5nU3Rv cmUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SURCQmFja2luZ1N0b3JlOjpvcGVuKToKKwogMjAx My0wMy0xMiAgU2hlcmlmZiBCb3QgIDx3ZWJraXQucmV2aWV3LmJvdEBnbWFpbC5jb20+CiAKICAg ICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjE0NTI4MC4KZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJDb3JlL01vZHVsZXMvaW5kZXhlZGRiL0lEQkJhY2tpbmdTdG9yZS5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9Nb2R1bGVzL2luZGV4ZWRkYi9JREJCYWNraW5nU3RvcmUuY3BwCmluZGV4IDBhYmNm ZWQ4NmM3NGE3MTVmZjY2NTYzMGU4MjNlZjdkNGI3NzRlZDQuLjFmOTlmMWEyOTllOTE5MTg1ZTJh Y2E3MDk3ZDJjMDMzMTM4OWE4NjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMv aW5kZXhlZGRiL0lEQkJhY2tpbmdTdG9yZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvTW9kdWxl cy9pbmRleGVkZGIvSURCQmFja2luZ1N0b3JlLmNwcApAQCAtMzY1LDYgKzM2NSw3IEBAIGVudW0g SURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5SZXN1bHQgewogICAgIElEQkxldmVsREJCYWNraW5n U3RvcmVPcGVuRmFpbGVkSU9FcnJDaGVja2luZ1NjaGVtYSwKICAgICBJREJMZXZlbERCQmFja2lu Z1N0b3JlT3BlbkZhaWxlZFVua25vd25FcnIsCiAgICAgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9w ZW5NZW1vcnlGYWlsZWQsCisgICAgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5BdHRlbXB0Tm9u QVNDSUksCiAgICAgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5NYXgsCiB9OwogCkBAIC0zOTEs NiArMzkyLDggQEAgUGFzc1JlZlB0cjxJREJCYWNraW5nU3RvcmU+IElEQkJhY2tpbmdTdG9yZTo6 b3BlbihTZWN1cml0eU9yaWdpbiogc2VjdXJpdHlPcmlnaW4KICAgICAgICAgfQogICAgICAgICBI aXN0b2dyYW1TdXBwb3J0OjpoaXN0b2dyYW1FbnVtZXJhdGlvbigiV2ViQ29yZS5JbmRleGVkREIu QmFja2luZ1N0b3JlLk9wZW5TdGF0dXMiLCBJREJMZXZlbERCQmFja2luZ1N0b3JlT3Blbk1lbW9y eVN1Y2Nlc3MsIElEQkxldmVsREJCYWNraW5nU3RvcmVPcGVuTWF4KTsKICAgICB9IGVsc2Ugewor ICAgICAgICBpZiAoIXBhdGhCYXNlLmNvbnRhaW5zT25seUFTQ0lJKCkpCisgICAgICAgICAgICBI aXN0b2dyYW1TdXBwb3J0OjpoaXN0b2dyYW1FbnVtZXJhdGlvbigiV2ViQ29yZS5JbmRleGVkREIu QmFja2luZ1N0b3JlLk9wZW5TdGF0dXMiLCBJREJMZXZlbERCQmFja2luZ1N0b3JlT3BlbkF0dGVt cHROb25BU0NJSSwgSURCTGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5NYXgpOwogICAgICAgICBpZiAo IW1ha2VBbGxEaXJlY3RvcmllcyhwYXRoQmFzZSkpIHsKICAgICAgICAgICAgIExPR19FUlJPUigi VW5hYmxlIHRvIGNyZWF0ZSBJbmRleGVkREIgZGF0YWJhc2UgcGF0aCAlcyIsIHBhdGhCYXNlLnV0 ZjgoKS5kYXRhKCkpOwogICAgICAgICAgICAgSGlzdG9ncmFtU3VwcG9ydDo6aGlzdG9ncmFtRW51 bWVyYXRpb24oIldlYkNvcmUuSW5kZXhlZERCLkJhY2tpbmdTdG9yZS5PcGVuU3RhdHVzIiwgSURC TGV2ZWxEQkJhY2tpbmdTdG9yZU9wZW5GYWlsZWREaXJlY3RvcnksIElEQkxldmVsREJCYWNraW5n U3RvcmVPcGVuTWF4KTsK