Otherwise we are managing frames and their resources using too many maps.
Created attachment 116504 [details] Patch
Comment on attachment 116504 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116504&action=review > Source/WebCore/inspector/Inspector.json:680 > + { "name": "frameId", "$ref": "FrameId", "description": "Frame identifier.", "hidden": true }, > + { "name": "loaderId", "$ref": "LoaderId", "description": "Loader identifier." }, These should be added to the end as optional right? Since this is a change from Inspector-0.1.json. Also why is frameId always hidden? > Source/WebCore/inspector/front-end/ApplicationCacheModel.js:59 > + var frame = /** @type {WebInspector.ResourceTreeFrame} */ event.data; These type comments are getting intrusive…
(In reply to comment #2) > (From update of attachment 116504 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116504&action=review > > > Source/WebCore/inspector/Inspector.json:680 > > + { "name": "frameId", "$ref": "FrameId", "description": "Frame identifier.", "hidden": true }, > > + { "name": "loaderId", "$ref": "LoaderId", "description": "Loader identifier." }, > > These should be added to the end as optional right? Since this is a change from Inspector-0.1.json. Also why is frameId always hidden? We can add any parameters into the _events_ and remove optional parameters from them. We can't add non-optional parameters to the _commands_ only. > > > Source/WebCore/inspector/front-end/ApplicationCacheModel.js:59 > > + var frame = /** @type {WebInspector.ResourceTreeFrame} */ event.data; > > These type comments are getting intrusive… I know. We are experimenting with this type of cast. It is not clear how much we need to annotate to get a strict type checking. But as is, compilation already catches a handful of bugs for me every day.
Comment on attachment 116504 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116504&action=review >>> Source/WebCore/inspector/front-end/ApplicationCacheModel.js:59 >>> + var frame = /** @type {WebInspector.ResourceTreeFrame} */ event.data; >> >> These type comments are getting intrusive… > > I know. We are experimenting with this type of cast. It is not clear how much we need to annotate to get a strict type checking. But as is, compilation already catches a handful of bugs for me every day. I'm confused. Why does order not matter for events when other clients might be using this and break after this change?
(In reply to comment #4) > (From update of attachment 116504 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116504&action=review > > >>> Source/WebCore/inspector/front-end/ApplicationCacheModel.js:59 > >>> + var frame = /** @type {WebInspector.ResourceTreeFrame} */ event.data; > >> > >> These type comments are getting intrusive… > > > > I know. We are experimenting with this type of cast. It is not clear how much we need to annotate to get a strict type checking. But as is, compilation already catches a handful of bugs for me every day. > > I'm confused. Why does order not matter for events when other clients might be using this and break after this change? Sigh, that is not the comment I replied to.
Comment on attachment 116504 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116504&action=review > Source/WebCore/inspector/front-end/ResourceTreeModel.js:108 > + this._frameDetached(this.mainFrame.id); Please move this to _frameNavigated. > Source/WebCore/inspector/front-end/ResourceTreeModel.js:117 > + _frameNavigated: function(framePayload) You should remove all child frames here.
Created attachment 116524 [details] [Patch] First pass of review comments addressed
Comment on attachment 116524 [details] [Patch] First pass of review comments addressed View in context: https://bugs.webkit.org/attachment.cgi?id=116524&action=review > Source/WebCore/inspector/front-end/ResourceTreeModel.js:154 > + if (!framePayload.parentId && !WebInspector.networkLog.resources.length) { Please comment on this logic.
Committed r101154: <http://trac.webkit.org/changeset/101154>