Summary: | Web Inspector: Exception under ContentViewContainer _disassociateFromContentView | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||
Component: | Web Inspector | Assignee: | Timothy Hatcher <timothy> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Timothy Hatcher
2015-05-03 07:43:25 PDT
Created attachment 252268 [details]
Patch
Comment on attachment 252268 [details] 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.) Comment on attachment 252268 [details] Patch Clearing flags on attachment: 252268 Committed r183733: <http://trac.webkit.org/changeset/183733> All reviewed patches have been landed. Closing bug. Comment on attachment 252268 [details] 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. |