132204 2014-04-25 14:14:38 -0700 REGRESSION (r167775): Safari crashes in ViewSnapshotStore::pruneSnapshots after loading 20 pages 2014-04-27 11:16:46 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton andersca dino mitz mrowe rniwa simon.fraser oldest_to_newest 1004199 0 thorton 2014-04-25 14:14:38 -0700 Typo (and also a separate mistake not causing a crash but causing the code to run more than it should). <rdar://problem/16729123> 1004204 1 230199 thorton 2014-04-25 14:21:27 -0700 Created attachment 230199 patch 1004206 2 230199 andersca 2014-04-25 14:24:08 -0700 Comment on attachment 230199 patch View in context: https://bugs.webkit.org/attachment.cgi?id=230199&action=review > Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:79 > + const auto& snapshot = m_snapshotMap.find(snapshotUUID); I don't think you should call this snapshot since it's an iterator. How about auto snapshotIt = m_snapshotMap.find(snapshotUUID) 1004210 3 thorton 2014-04-25 14:29:22 -0700 https://trac.webkit.org/changeset/167822 1004351 4 thorton 2014-04-26 15:28:38 -0700 This didn't fix the problem 100%. 1004352 5 230245 thorton 2014-04-26 15:33:19 -0700 Created attachment 230245 followup 1004368 6 230245 mitz 2014-04-26 19:37:21 -0700 Comment on attachment 230245 followup View in context: https://bugs.webkit.org/attachment.cgi?id=230245&action=review > Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:113 > + if (snapshotIter == m_snapshotMap.end()) { > + ASSERT_NOT_REACHED(); > + return; > + } This is better written as ASSERT(snapshotIter != m_snapshotMap.end()); if (snapshowIter == m_snapshotMap.end()) return; Since the assertion failure message will say what is the condition that was false. 1004369 7 thorton 2014-04-26 19:45:17 -0700 Sam thinks that the early return is sloppy and that we should just keep crashing if it's wrong so that we fix it; http://trac.webkit.org/changeset/167849. 1004420 8 230245 darin 2014-04-27 11:16:46 -0700 Comment on attachment 230245 followup View in context: https://bugs.webkit.org/attachment.cgi?id=230245&action=review > Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:109 > + const auto& snapshotIter = m_snapshotMap.find(oldestSnapshotUUID); Why const auto& instead of auto? 230199 2014-04-25 14:21:27 -0700 2014-04-25 14:24:08 -0700 patch prune.diff text/plain 4255 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA5MTk3NTE1Li43YzU1MjA0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg QEAKKzIwMTQtMDQtMjUgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgUkVHUkVTU0lPTiAocjE2Nzc3NSk6IFNhZmFyaSBjcmFzaGVzIGluIFZpZXdTbmFw c2hvdFN0b3JlOjpwcnVuZVNuYXBzaG90cyBhZnRlciBsb2FkaW5nIDIwIHBhZ2VzCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzIyMDQKKyAgICAgICAg PHJkYXI6Ly9wcm9ibGVtLzE2NzI5MTIzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgICogVUlQcm9jZXNzL21hYy9WaWV3U25hcHNob3RTdG9yZS5oOgor ICAgICAgICAqIFVJUHJvY2Vzcy9tYWMvVmlld1NuYXBzaG90U3RvcmUubW06CisgICAgICAgIChX ZWJLaXQ6OlZpZXdTbmFwc2hvdFN0b3JlOjpWaWV3U25hcHNob3RTdG9yZSk6CisgICAgICAgIChX ZWJLaXQ6OlZpZXdTbmFwc2hvdFN0b3JlOjpwcnVuZVNuYXBzaG90cyk6CisgICAgICAgIChXZWJL aXQ6OlZpZXdTbmFwc2hvdFN0b3JlOjpyZWNvcmRTbmFwc2hvdCk6CisgICAgICAgIEtlZXAgdHJh Y2sgb2YgdGhlIG51bWJlciBvZiBzbmFwc2hvdHMgdGhhdCBhY3R1YWxseSBoYXZlIGxpdmUgaW1h Z2VzOyB0aGUKKyAgICAgICAgc2l6ZSBvZiB0aGUgc25hcHNob3QgbWFwIG5vIGxvbmdlciByZXBy ZXNlbnRzIHRoYXQuCisgICAgICAgIEFsc28sIGZpeCB0aGUgY3Jhc2ggYnkgdXNpbmcgdGhlIFVV SUQgZnJvbSB0aGUgY3VycmVudCBpdGVtIGluc3RlYWQgb2YgZnJvbQorICAgICAgICB0aGUgKHBv dGVudGlhbGx5IG51bGwpIG1vc3QgZGlzdGFudCBpdGVtOyByMTY3Nzc1IGFjY2lkZW50YWxseSB0 eXBvJ2QgdGhpcy4KKwogMjAxNC0wNC0yNSAgQ2FybG9zIEdhcmNpYSBDYW1wb3MgIDxjZ2FyY2lh QGlnYWxpYS5jb20+CiAKICAgICAgICAgW0dUS10gUGx1Z2luIHByb2Nlc3MgY3Jhc2hlcyB3aXRo IEdUSzIgd2luZG93ZWQgcGx1Z2lucwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL21hYy9WaWV3U25hcHNob3RTdG9yZS5oIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21h Yy9WaWV3U25hcHNob3RTdG9yZS5oCmluZGV4IDFmMDI3NDEuLjA1MTNmNDEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvVmlld1NuYXBzaG90U3RvcmUuaAorKysgYi9T b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1ZpZXdTbmFwc2hvdFN0b3JlLmgKQEAgLTcyLDYg KzcyLDcgQEAgcHJpdmF0ZToKICAgICBIYXNoTWFwPFN0cmluZywgU25hcHNob3Q+IG1fc25hcHNo b3RNYXA7CiAKICAgICBib29sIG1fZW5hYmxlZDsKKyAgICB1bnNpZ25lZCBtX3NuYXBzaG90c1dp dGhJbWFnZXNDb3VudDsKIH07CiAKIH0gLy8gbmFtZXNwYWNlIFdlYktpdApkaWZmIC0tZ2l0IGEv U291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21hYy9WaWV3U25hcHNob3RTdG9yZS5tbSBiL1NvdXJj ZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvVmlld1NuYXBzaG90U3RvcmUubW0KaW5kZXggMTkxMTYx NC4uNDA5ODQ0MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21hYy9WaWV3 U25hcHNob3RTdG9yZS5tbQorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1ZpZXdT bmFwc2hvdFN0b3JlLm1tCkBAIC00MCw2ICs0MCw3IEBAIG5hbWVzcGFjZSBXZWJLaXQgewogCiBW aWV3U25hcHNob3RTdG9yZTo6Vmlld1NuYXBzaG90U3RvcmUoKQogICAgIDogbV9lbmFibGVkKHRy dWUpCisgICAgLCBtX3NuYXBzaG90c1dpdGhJbWFnZXNDb3VudCgwKQogewogfQogCkBAIC01NSwx MiArNTYsMTIgQEAgVmlld1NuYXBzaG90U3RvcmUmIFZpZXdTbmFwc2hvdFN0b3JlOjpzaGFyZWQo KQogCiB2b2lkIFZpZXdTbmFwc2hvdFN0b3JlOjpwcnVuZVNuYXBzaG90cyhXZWJQYWdlUHJveHkm IHdlYlBhZ2VQcm94eSkKIHsKLSAgICBpZiAobV9zbmFwc2hvdE1hcC5zaXplKCkgPD0gbWF4aW11 bVNuYXBzaG90Q291bnQpCisgICAgaWYgKG1fc25hcHNob3RzV2l0aEltYWdlc0NvdW50IDw9IG1h eGltdW1TbmFwc2hvdENvdW50KQogICAgICAgICByZXR1cm47CiAKICAgICB1aW50MzJfdCBjdXJy ZW50SW5kZXggPSB3ZWJQYWdlUHJveHkuYmFja0ZvcndhcmRMaXN0KCkuY3VycmVudEluZGV4KCk7 CiAgICAgdWludDMyX3QgbWF4RGlzdGFuY2UgPSAwOwotICAgIFdlYkJhY2tGb3J3YXJkTGlzdEl0 ZW0qIG1vc3REaXN0YW50U25hcHNob3R0ZWRJdGVtID0gbnVsbHB0cjsKKyAgICBhdXRvIG1vc3RE aXN0YW50U25hcHNob3QgPSBtX3NuYXBzaG90TWFwLmVuZCgpOwogICAgIGF1dG8gYmFja0Zvcndh cmRFbnRyaWVzID0gd2ViUGFnZVByb3h5LmJhY2tGb3J3YXJkTGlzdCgpLmVudHJpZXMoKTsKIAog ICAgIC8vIEZpcnN0LCB0cnkgdG8gZXZpY3QgdGhlIHNuYXBzaG90IGZvciB0aGUgcGFnZSBmYXJ0 aGVzdCBmcm9tIHRoZSBjdXJyZW50IGJhY2stZm9yd2FyZCBpdGVtLgpAQCAtNzUsNyArNzYsNyBA QCB2b2lkIFZpZXdTbmFwc2hvdFN0b3JlOjpwcnVuZVNuYXBzaG90cyhXZWJQYWdlUHJveHkmIHdl YlBhZ2VQcm94eSkKICAgICAgICAgaWYgKHNuYXBzaG90VVVJRC5pc0VtcHR5KCkpCiAgICAgICAg ICAgICBjb250aW51ZTsKIAotICAgICAgICBjb25zdCBhdXRvJiBzbmFwc2hvdCA9IG1fc25hcHNo b3RNYXAuZmluZChtb3N0RGlzdGFudFNuYXBzaG90dGVkSXRlbS0+c25hcHNob3RVVUlEKCkpOwor ICAgICAgICBjb25zdCBhdXRvJiBzbmFwc2hvdCA9IG1fc25hcHNob3RNYXAuZmluZChzbmFwc2hv dFVVSUQpOwogICAgICAgICBpZiAoc25hcHNob3QgPT0gbV9zbmFwc2hvdE1hcC5lbmQoKSkKICAg ICAgICAgICAgIGNvbnRpbnVlOwogCkBAIC04MywxMyArODQsMTMgQEAgdm9pZCBWaWV3U25hcHNo b3RTdG9yZTo6cHJ1bmVTbmFwc2hvdHMoV2ViUGFnZVByb3h5JiB3ZWJQYWdlUHJveHkpCiAgICAg ICAgIGlmICghc25hcHNob3QtPnZhbHVlLmhhc0ltYWdlKCkpCiAgICAgICAgICAgICBjb250aW51 ZTsKIAotICAgICAgICBtb3N0RGlzdGFudFNuYXBzaG90dGVkSXRlbSA9IGl0ZW07CisgICAgICAg IG1vc3REaXN0YW50U25hcHNob3QgPSBzbmFwc2hvdDsKICAgICAgICAgbWF4RGlzdGFuY2UgPSBk aXN0YW5jZTsKICAgICB9CiAKLSAgICBpZiAobW9zdERpc3RhbnRTbmFwc2hvdHRlZEl0ZW0pIHsK LSAgICAgICAgY29uc3QgYXV0byYgc25hcHNob3QgPSBtX3NuYXBzaG90TWFwLmZpbmQobW9zdERp c3RhbnRTbmFwc2hvdHRlZEl0ZW0tPnNuYXBzaG90VVVJRCgpKTsKLSAgICAgICAgc25hcHNob3Qt PnZhbHVlLmNsZWFySW1hZ2UoKTsKKyAgICBpZiAobW9zdERpc3RhbnRTbmFwc2hvdCAhPSBtX3Nu YXBzaG90TWFwLmVuZCgpKSB7CisgICAgICAgIG1vc3REaXN0YW50U25hcHNob3QtPnZhbHVlLmNs ZWFySW1hZ2UoKTsKKyAgICAgICAgbV9zbmFwc2hvdHNXaXRoSW1hZ2VzQ291bnQtLTsKICAgICAg ICAgcmV0dXJuOwogICAgIH0KIApAQCAtMTA3LDYgKzEwOCw3IEBAIHZvaWQgVmlld1NuYXBzaG90 U3RvcmU6OnBydW5lU25hcHNob3RzKFdlYlBhZ2VQcm94eSYgd2ViUGFnZVByb3h5KQogCiAgICAg Y29uc3QgYXV0byYgc25hcHNob3QgPSBtX3NuYXBzaG90TWFwLmZpbmQob2xkZXN0U25hcHNob3RV VUlEKTsKICAgICBzbmFwc2hvdC0+dmFsdWUuY2xlYXJJbWFnZSgpOworICAgIG1fc25hcHNob3Rz V2l0aEltYWdlc0NvdW50LS07CiB9CiAKICNpZiBVU0UoSU9TVVJGQUNFKQpAQCAtMTU4LDYgKzE2 MCw5IEBAIHZvaWQgVmlld1NuYXBzaG90U3RvcmU6OnJlY29yZFNuYXBzaG90KFdlYlBhZ2VQcm94 eSYgd2ViUGFnZVByb3h5KQogI2VuZGlmCiAKICAgICBtX3NuYXBzaG90TWFwLmFkZChpdGVtLT5z bmFwc2hvdFVVSUQoKSwgc25hcHNob3QpOworCisgICAgaWYgKHNuYXBzaG90Lmhhc0ltYWdlKCkp CisgICAgICAgIG1fc25hcHNob3RzV2l0aEltYWdlc0NvdW50Kys7CiB9CiAKIGJvb2wgVmlld1Nu YXBzaG90U3RvcmU6OmdldFNuYXBzaG90KFdlYkJhY2tGb3J3YXJkTGlzdEl0ZW0qIGl0ZW0sIFZp ZXdTbmFwc2hvdFN0b3JlOjpTbmFwc2hvdCYgc25hcHNob3QpCg== 230245 2014-04-26 15:33:19 -0700 2014-04-27 11:16:46 -0700 followup crash.diff text/plain 2845 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBiOTQ2YjA3Li4xNzVhMjc3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkg QEAKKzIwMTQtMDQtMjYgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgUkVHUkVTU0lPTiAocjE2Nzc3NSk6IFNhZmFyaSBjcmFzaGVzIGluIFZpZXdTbmFw c2hvdFN0b3JlOjpwcnVuZVNuYXBzaG90cyBhZnRlciBsb2FkaW5nIDIwIHBhZ2VzCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzIyMDQKKyAgICAgICAg PHJkYXI6Ly9wcm9ibGVtLzE2NzM1NjIyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgICogVUlQcm9jZXNzL21hYy9WaWV3U25hcHNob3RTdG9yZS5tbToK KyAgICAgICAgKFdlYktpdDo6Vmlld1NuYXBzaG90U3RvcmU6OnBydW5lU25hcHNob3RzKToKKyAg ICAgICAgKFdlYktpdDo6Vmlld1NuYXBzaG90U3RvcmU6OnJlY29yZFNuYXBzaG90KToKKyAgICAg ICAgRml4IGEgYnVnIHdoZXJlIHRoZSBjb3VudCBvZiBzbmFwc2hvdHMgd2l0aCBsaXZlIGltYWdl cyB3YXMgdG9vIGhpZ2gKKyAgICAgICAgYmVjYXVzZSB3ZSB3ZXJlIGZhaWxpbmcgdG8gZGVjcmVt ZW50IGl0IHdoZW4gcmVwbGFjaW5nIGEgc25hcHNob3Qgb2YKKyAgICAgICAgYW4gZXhpc3Rpbmcg aXRlbSB3aXRoIGEgZnJlc2ggb25lLgorICAgICAgICBBbHNvLCBhdm9pZCBjcmFzaGluZyAoZXhj ZXB0IGluIGRlYnVnKSB3aGVuIHdlIGdldCB0aGUgY291bnQgd3JvbmcuCisKIDIwMTQtMDQtMjUg IFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVT U0lPTiAocjE2Nzc3NSk6IFNhZmFyaSBjcmFzaGVzIGluIFZpZXdTbmFwc2hvdFN0b3JlOjpwcnVu ZVNuYXBzaG90cyBhZnRlciBsb2FkaW5nIDIwIHBhZ2VzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi S2l0Mi9VSVByb2Nlc3MvbWFjL1ZpZXdTbmFwc2hvdFN0b3JlLm1tIGIvU291cmNlL1dlYktpdDIv VUlQcm9jZXNzL21hYy9WaWV3U25hcHNob3RTdG9yZS5tbQppbmRleCAxM2FkOWFiLi43YzIyZjE4 IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1ZpZXdTbmFwc2hvdFN0 b3JlLm1tCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvVmlld1NuYXBzaG90U3Rv cmUubW0KQEAgLTEwMCwxNCArMTAwLDE4IEBAIHZvaWQgVmlld1NuYXBzaG90U3RvcmU6OnBydW5l U25hcHNob3RzKFdlYlBhZ2VQcm94eSYgd2ViUGFnZVByb3h5KQogICAgIFN0cmluZyBvbGRlc3RT bmFwc2hvdFVVSUQ7CiAKICAgICBmb3IgKGNvbnN0IGF1dG8mIHV1aWRBbmRTbmFwc2hvdCA6IG1f c25hcHNob3RNYXApIHsKLSAgICAgICAgaWYgKHV1aWRBbmRTbmFwc2hvdC52YWx1ZS5jcmVhdGlv blRpbWUgPCBvbGRlc3RTbmFwc2hvdFRpbWUpIHsKKyAgICAgICAgaWYgKHV1aWRBbmRTbmFwc2hv dC52YWx1ZS5jcmVhdGlvblRpbWUgPCBvbGRlc3RTbmFwc2hvdFRpbWUgJiYgdXVpZEFuZFNuYXBz aG90LnZhbHVlLmhhc0ltYWdlKCkpIHsKICAgICAgICAgICAgIG9sZGVzdFNuYXBzaG90VGltZSA9 IHV1aWRBbmRTbmFwc2hvdC52YWx1ZS5jcmVhdGlvblRpbWU7CiAgICAgICAgICAgICBvbGRlc3RT bmFwc2hvdFVVSUQgPSB1dWlkQW5kU25hcHNob3Qua2V5OwogICAgICAgICB9CiAgICAgfQogCi0g ICAgY29uc3QgYXV0byYgc25hcHNob3QgPSBtX3NuYXBzaG90TWFwLmZpbmQob2xkZXN0U25hcHNo b3RVVUlEKTsKLSAgICBzbmFwc2hvdC0+dmFsdWUuY2xlYXJJbWFnZSgpOworICAgIGNvbnN0IGF1 dG8mIHNuYXBzaG90SXRlciA9IG1fc25hcHNob3RNYXAuZmluZChvbGRlc3RTbmFwc2hvdFVVSUQp OworICAgIGlmIChzbmFwc2hvdEl0ZXIgPT0gbV9zbmFwc2hvdE1hcC5lbmQoKSkgeworICAgICAg ICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKKyAgICAgICAgcmV0dXJuOworICAgIH0KKyAgICBzbmFw c2hvdEl0ZXItPnZhbHVlLmNsZWFySW1hZ2UoKTsKICAgICBtX3NuYXBzaG90c1dpdGhJbWFnZXND b3VudC0tOwogfQogCkBAIC0xNDMsOCArMTQ3LDE0IEBAIHZvaWQgVmlld1NuYXBzaG90U3RvcmU6 OnJlY29yZFNuYXBzaG90KFdlYlBhZ2VQcm94eSYgd2ViUGFnZVByb3h5KQogICAgIHBydW5lU25h cHNob3RzKHdlYlBhZ2VQcm94eSk7CiAKICAgICBTdHJpbmcgb2xkU25hcHNob3RVVUlEID0gaXRl bS0+c25hcHNob3RVVUlEKCk7Ci0gICAgaWYgKCFvbGRTbmFwc2hvdFVVSUQuaXNFbXB0eSgpKQot ICAgICAgICBtX3NuYXBzaG90TWFwLnJlbW92ZShvbGRTbmFwc2hvdFVVSUQpOworICAgIGlmICgh b2xkU25hcHNob3RVVUlELmlzRW1wdHkoKSkgeworICAgICAgICBjb25zdCBhdXRvJiBvbGRTbmFw c2hvdEl0ZXIgPSBtX3NuYXBzaG90TWFwLmZpbmQob2xkU25hcHNob3RVVUlEKTsKKyAgICAgICAg aWYgKG9sZFNuYXBzaG90SXRlciAhPSBtX3NuYXBzaG90TWFwLmVuZCgpKSB7CisgICAgICAgICAg ICBpZiAob2xkU25hcHNob3RJdGVyLT52YWx1ZS5oYXNJbWFnZSgpKQorICAgICAgICAgICAgICAg IG1fc25hcHNob3RzV2l0aEltYWdlc0NvdW50LS07CisgICAgICAgICAgICBtX3NuYXBzaG90TWFw LnJlbW92ZShvbGRTbmFwc2hvdEl0ZXIpOworICAgICAgICB9CisgICAgfQogCiAgICAgaXRlbS0+ c2V0U25hcHNob3RVVUlEKGNyZWF0ZUNhbm9uaWNhbFVVSURTdHJpbmcoKSk7CiAgICAgCg==