144546 2015-05-03 07:43:25 -0700 Web Inspector: Exception under ContentViewContainer _disassociateFromContentView 2015-05-03 18:07:47 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 timothy timothy commit-queue graouts joepeck jonowells mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1091213 0 timothy 2015-05-03 07:43:25 -0700 _disassociateFromContentView is being passed a BackForwardEntry instead of a ContentView in one case. There is also an associated logic error. I think this was masked by the fact that _disassociateFromContentView wouldn't call closed() if the representedObject was null on the passed object. Since BackForwardEntry doesn't have a representedObject, it would always abort early. This would lead to memory leaks too. 1091214 1 webkit-bug-importer 2015-05-03 07:43:41 -0700 <rdar://problem/20793755> 1091215 2 252268 timothy 2015-05-03 07:46:00 -0700 Created attachment 252268 Patch 1091262 3 252268 burg 2015-05-03 15:10:13 -0700 Comment on attachment 252268 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252268&action=review r=me > Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:182 > + this._disassociateFromContentView(removedEntries[i].contentView); Would be prudent to add a type test for the method's parameter. (The method name is really clear, but this still slipped up.) 1091274 4 252268 commit-queue 2015-05-03 15:56:27 -0700 Comment on attachment 252268 Patch Clearing flags on attachment: 252268 Committed r183733: <http://trac.webkit.org/changeset/183733> 1091275 5 commit-queue 2015-05-03 15:56:31 -0700 All reviewed patches have been landed. Closing bug. 1091304 6 252268 timothy 2015-05-03 18:07:47 -0700 Comment on attachment 252268 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252268&action=review >> Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:182 >> + this._disassociateFromContentView(removedEntries[i].contentView); > > Would be prudent to add a type test for the method's parameter. (The method name is really clear, but this still slipped up.) I think this happened during refactoring when you added BackForwardEntry into the mix instead of dealing with ContentViews directly. It was just masked by an early return that I recently removed. But yes, an assert for the type would normally be good. 252268 2015-05-03 07:46:00 -0700 2015-05-03 15:56:27 -0700 Patch bug-144546-20150503074447.patch text/plain 2256 timothy U3VidmVyc2lvbiBSZXZpc2lvbjogMTgzNzI1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCAy OGU0YWUxN2U0MTJlNDI3OTA2OWZhMzRhOGY4MDAzZmIyNWJkNTY3Li5iYzQwYjIwMDdlMWNjZDc4 ZDc3ZGY2NjlmNTJkMzk0NTRiZTA3OTQ3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxNS0wNS0wMyAgVGltb3RoeSBIYXRjaGVyICA8dGltb3RoeUBhcHBsZS5j b20+CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogRXhjZXB0aW9uIHVuZGVyIENvbnRlbnRWaWV3 Q29udGFpbmVyIF9kaXNhc3NvY2lhdGVGcm9tQ29udGVudFZpZXcKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0NDU0NgorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogVXNlckludGVyZmFjZS9WaWV3cy9Db250 ZW50Vmlld0NvbnRhaW5lci5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0Nv bnRhaW5lci5wcm90b3R5cGUuc2hvd0NvbnRlbnRWaWV3KToKKyAgICAgICAgTWFrZSBzdXJlIHRv IHBhc3MgdGhlIENvbnRlbnRWaWV3IHRvIF9kaXNhc3NvY2lhdGVGcm9tQ29udGVudFZpZXcuIEFs c28gbmVnYXRlIHRoZSByZXN1bHQKKyAgICAgICAgb2YgdGhlIF9iYWNrRm9yd2FyZExpc3Quc29t ZSgpLCBzaW5jZSB3ZSBkb24ndCB3YW50IHRvIGRpc3NvY2lhdGUgaWYgdGhlIGNvbnRlbnQgdmll dyBpcworICAgICAgICBzdGlsbCBpbiB0aGUgYmFjay1mb3J3YXJkIGxpc3QuCisKIDIwMTUtMDUt MDIgIFRpbW90aHkgSGF0Y2hlciAgPHRpbW90aHlAYXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJ bnNwZWN0b3I6IEFsbG93IGNsb3NpbmcgYW5kIHJlb3BlbmluZyB0aGUgRGVidWdnZXIgdGFiCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9Db250 ZW50Vmlld0NvbnRhaW5lci5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNl L1ZpZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVyLmpzCmluZGV4IGRkOWM3MTY2Zjk1MGViOTEzOTQw ZTQ3MGVjMDJlM2IzNzM2ZGNkOGQuLmViM2I1ZjMzNzA5MzM0YmUyODMwZDkzMDNhZmQ2M2FjYzQ5 MTk4NzcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1Zp ZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVyLmpzCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9V c2VySW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVyLmpzCkBAIC0xNzQsMTEgKzE3 NCwxMiBAQCBXZWJJbnNwZWN0b3IuQ29udGVudFZpZXdDb250YWluZXIucHJvdG90eXBlID0gewog ICAgICAgICAvLyBEaXNhc3NvY2lhdGUgd2l0aCB0aGUgcmVtb3ZlZCBjb250ZW50IHZpZXdzLgog ICAgICAgICBmb3IgKHZhciBpID0gMDsgaSA8IHJlbW92ZWRFbnRyaWVzLmxlbmd0aDsgKytpKSB7 CiAgICAgICAgICAgICAvLyBTa2lwIGRpc2Fzc29jaWF0aW9uIGlmIHRoaXMgY29udGVudCB2aWV3 IGlzIHN0aWxsIGluIHRoZSBiYWNrL2ZvcndhcmQgbGlzdC4KLSAgICAgICAgICAgIHZhciBzaG91 bGREaXNzb2NpYXRlQ29udGVudFZpZXcgPSB0aGlzLl9iYWNrRm9yd2FyZExpc3Quc29tZShmdW5j dGlvbihleGlzdGluZ0VudHJ5KSB7CisgICAgICAgICAgICB2YXIgc2hvdWxkRGlzc29jaWF0ZUNv bnRlbnRWaWV3ID0gIXRoaXMuX2JhY2tGb3J3YXJkTGlzdC5zb21lKGZ1bmN0aW9uKGV4aXN0aW5n RW50cnkpIHsKICAgICAgICAgICAgICAgICByZXR1cm4gZXhpc3RpbmdFbnRyeS5jb250ZW50Vmll dyA9PT0gcmVtb3ZlZEVudHJpZXNbaV0uY29udGVudFZpZXc7CiAgICAgICAgICAgICB9KTsKKwog ICAgICAgICAgICAgaWYgKHNob3VsZERpc3NvY2lhdGVDb250ZW50VmlldykKLSAgICAgICAgICAg ICAgICB0aGlzLl9kaXNhc3NvY2lhdGVGcm9tQ29udGVudFZpZXcocmVtb3ZlZEVudHJpZXNbaV0p OworICAgICAgICAgICAgICAgIHRoaXMuX2Rpc2Fzc29jaWF0ZUZyb21Db250ZW50VmlldyhyZW1v dmVkRW50cmllc1tpXS5jb250ZW50Vmlldyk7CiAgICAgICAgIH0KIAogICAgICAgICAvLyBBc3Nv Y2lhdGUgd2l0aCB0aGUgbmV3IGNvbnRlbnQgdmlldy4K