106435 2013-01-09 03:10:41 -0800 Web Inspector: add a new protocol method to InspectorDOMAgent to be able to request child nodes at a given depth 2013-01-10 09:34:04 -0800 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED DUPLICATE 66868 InRadar P2 Normal --- 66868 1 graouts graouts apavlov joepeck keishi loislo pfeldman pmuellr timothy vsevik web-inspector-bugs webkit-bug-importer yurys oldest_to_newest 803185 0 graouts 2013-01-09 03:10:41 -0800 In order to facilitate true deep expansion of nodes in the DOM tree (see https://bugs.webkit.org/show_bug.cgi?id=66868), we should be able to pass an extra parameter to InspectorDOMAgent::requestChildNodes(), and subsequently pushChildNodesToFrontend(), to specify the depth at which we want to gather children of a given node. This should be relatively easy since the method buildArrayForContainerChildren() already has support for specifying the depth and pushChildNodesToFrontend() relies on this method to gather the child nodes. 803186 1 webkit-bug-importer 2013-01-09 03:11:09 -0800 <rdar://problem/12980145> 803194 2 graouts 2013-01-09 03:21:53 -0800 While it would be simpler to just add an extra parameter to existing methods, the fact that in the DOM API a node's .childNodes are always the immediate descendant might prompt us to actually introduce other method with naming that might more appropriate. I'd like to hear people's suggestions on the topic. 803481 3 joepeck 2013-01-09 10:45:48 -0800 Ahh, so you think it would be clearer to have a function requestChildNodesWithDepth instead of requestChildNodes(depth). That seems fine to me. I don't have a preference. 803696 4 181982 graouts 2013-01-09 14:18:06 -0800 Created attachment 181982 Patch 803710 5 joepeck 2013-01-09 14:37:54 -0800 View in context: https://bugs.webkit.org/attachment.cgi?id=181982&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:488 > + pushChildNodesToFrontend(nodeId, depth); You should produce an error or handle the case of depth <= 0. Negative doesn't make sense, and it looks like 0 is a special case internally to only get the text node children. Also document this in the protocol file. An existing example would be setDeviceMetricsOverride. It documents its range in Inspector.json and produces an error for negative values. You could also add an ASSERT in buildArrayForContainerChildren that depth is >= 0. 803713 6 181982 joepeck 2013-01-09 14:38:38 -0800 Comment on attachment 181982 Patch r=me with handling <= 0 803716 7 joepeck 2013-01-09 14:39:48 -0800 You may also want to update the title of the bug + thus ChangeLog. Since now you're not adding a param, you're adding a new function. 803723 8 181982 joepeck 2013-01-09 14:43:30 -0800 Comment on attachment 181982 Patch Actually, this probably requires at least a basic test. r- for an updated patch containing a test. 804290 9 181982 pfeldman 2013-01-10 05:41:09 -0800 Comment on attachment 181982 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181982&action=review You need a test and a corresponding front-end fix that would expand the entire branch. Exposing capabilities / fixing bugs for proprietary front-ends and not fixing it upstream is a no go. > Source/WebCore/inspector/Inspector.json:1715 > + "name": "requestChildNodesWithDepth", You should not add public methods unless you are committing to supporting them. > Source/WebCore/inspector/Inspector.json:1718 > + { "name": "depth", "type": "integer", "description": "The maximum depth at which children should be retrieved." } This should be an optional parameter of the requestChildNodes above. 804296 10 graouts 2013-01-10 05:48:33 -0800 (In reply to comment #9) > (From update of attachment 181982 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181982&action=review > > You need a test and a corresponding front-end fix that would expand the entire branch. Exposing capabilities / fixing bugs for proprietary front-ends and not fixing it upstream is a no go. > > > Source/WebCore/inspector/Inspector.json:1715 > > + "name": "requestChildNodesWithDepth", > > You should not add public methods unless you are committing to supporting them. > > > Source/WebCore/inspector/Inspector.json:1718 > > + { "name": "depth", "type": "integer", "description": "The maximum depth at which children should be retrieved." } > > This should be an optional parameter of the requestChildNodes above. Yes, that's what I'm going to do. I may close this bug as a DUPLICATE and fix https://bugs.webkit.org/show_bug.cgi?id=66868 instead. 804339 11 graouts 2013-01-10 06:56:02 -0800 Making the discussed backend enhancement as well as front-end fixes in https://bugs.webkit.org/show_bug.cgi?id=66868. *** This bug has been marked as a duplicate of bug 66868 *** 804449 12 joepeck 2013-01-10 09:34:04 -0800 (In reply to comment #9) > (From update of attachment 181982 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181982&action=review > > > Source/WebCore/inspector/Inspector.json:1718 > > + { "name": "depth", "type": "integer", "description": "The maximum depth at which children should be retrieved." } > > This should be an optional parameter of the requestChildNodes above. Yes, Antoine and I had an IRC discussion about this after the review and we came to the same conclusion. I forgot to comment here. 181982 2013-01-09 14:18:06 -0800 2013-01-10 05:41:09 -0800 Patch bug-106435-20130109231506.patch text/plain 5750 graouts U3VidmVyc2lvbiBSZXZpc2lvbjogMTM5MTgyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTc3MWY2YTNkZDZjNTY2 ZjBkZDMxZTI1OWRmNmI4YmYyMWNjNjA4My4uMjIxNDM5YzFlYTNkM2IxZjUyMWVjMzkyNmM2OTAy MzgzNmQ2ZWI5MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI0IEBACisyMDEzLTAxLTA5ICBBbnRv aW5lIFF1aW50ICA8Z3Jhb3V0c0BhcHBsZS5jb20+CisKKyAgICAgICAgV2ViIEluc3BlY3Rvcjog YWRkIGEgbmV3IGRlcHRoIHBhcmFtZXRlciB0byBJbnNwZWN0b3JET01BZ2VudDo6cmVxdWVzdENo aWxkTm9kZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTEwNjQzNQorCisgICAgICAgIEFkZCBhIG5ldyBwcm90b2NvbCBtZXRob2QgcmVxdWVzdENoaWxk Tm9kZXNXaXRoRGVwdGgoZGVwdGgpIHRvIGFsbG93IHRoZSBmcm9udC1lbmQgdG8KKyAgICAgICAg b2J0YWluIGNoaWxkcmVuIG9mIGEgbm9kZSBhdCBhbnkgZGVwdGggaW4gb25lIG1ldGhvZCBjYWxs LiBJbiBvcmRlciB0byBmYWNpbGl0YXRlIHRoaXMsCisgICAgICAgIHdlIGFkZGVkIGEgbmV3LCBv cHRpb25hbCwgImRlcHRoIiBwYXJhbWV0ZXIgdG8gdGhlIHByaXZhdGUgbWV0aG9kIHB1c2hDaGls ZE5vZGVzVG9Gcm9udGVuZCgpCisgICAgICAgIGFsbG93aW5nIHRvIGNhbGwgaW50byBidWlsZEFy cmF5Rm9yQ29udGFpbmVyQ2hpbGRyZW4oKSB3aXRoIHRoZSBkZXB0aCBwcm92aWRlZCB0bworICAg ICAgICByZXF1ZXN0Q2hpbGROb2Rlc1dpdGhEZXB0aCgpLgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogaW5zcGVjdG9yL0luc3BlY3Rvci5qc29uOgor ICAgICAgICAqIGluc3BlY3Rvci9JbnNwZWN0b3JET01BZ2VudC5jcHA6CisgICAgICAgIChXZWJD b3JlOjpJbnNwZWN0b3JET01BZ2VudDo6cHVzaENoaWxkTm9kZXNUb0Zyb250ZW5kKToKKyAgICAg ICAgKFdlYkNvcmU6Okluc3BlY3RvckRPTUFnZW50OjpyZXF1ZXN0Q2hpbGROb2Rlc1dpdGhEZXB0 aCk6CisgICAgICAgIChXZWJDb3JlKToKKyAgICAgICAgKiBpbnNwZWN0b3IvSW5zcGVjdG9yRE9N QWdlbnQuaDoKKyAgICAgICAgKEluc3BlY3RvckRPTUFnZW50KToKKwogMjAxMy0wMS0wOSAgWmVu byBBbGJpc3NlciAgPHplbm9Ad2Via2l0Lm9yZz4KIAogICAgICAgICBbUXRdW01hY10gR3JhcGhp Y3NTdXJmYWNlIGRvZXMgbm90IG5lZWQgZ2xFbmFibGUvZ2xEaXNhYmxlIGZvciB0ZXh0dXJlIHRh cmdldHMuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yLmpz b24gYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yLmpzb24KaW5kZXggNTdmNmY3 YWNkYmZkZWQ5MWZhNzk1OTJlNTkwY2I4ODkyNTdjZmFmOC4uZTA5NjlkOTczNzA4MWY2NTk5ZGY2 ZDE5OTdiMWVjMjk2ZjI0ODZjOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9y L0luc3BlY3Rvci5qc29uCisrKyBiL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3Iu anNvbgpAQCAtMTcxMiw2ICsxNzEyLDE0IEBACiAgICAgICAgICAgICAgICAgImRlc2NyaXB0aW9u IjogIlJlcXVlc3RzIHRoYXQgY2hpbGRyZW4gb2YgdGhlIG5vZGUgd2l0aCBnaXZlbiBpZCBhcmUg cmV0dXJuZWQgdG8gdGhlIGNhbGxlciBpbiBmb3JtIG9mIDxjb2RlPnNldENoaWxkTm9kZXM8L2Nv ZGU+IGV2ZW50cy4iCiAgICAgICAgICAgICB9LAogICAgICAgICAgICAgeworICAgICAgICAgICAg ICAgICJuYW1lIjogInJlcXVlc3RDaGlsZE5vZGVzV2l0aERlcHRoIiwKKyAgICAgICAgICAgICAg ICAicGFyYW1ldGVycyI6IFsKKyAgICAgICAgICAgICAgICAgICAgeyAibmFtZSI6ICJub2RlSWQi LCAiJHJlZiI6ICJOb2RlSWQiLCAiZGVzY3JpcHRpb24iOiAiSWQgb2YgdGhlIG5vZGUgdG8gZ2V0 IGNoaWxkcmVuIGZvci4iIH0sCisgICAgICAgICAgICAgICAgICAgIHsgIm5hbWUiOiAiZGVwdGgi LCAidHlwZSI6ICJpbnRlZ2VyIiwgImRlc2NyaXB0aW9uIjogIlRoZSBtYXhpbXVtIGRlcHRoIGF0 IHdoaWNoIGNoaWxkcmVuIHNob3VsZCBiZSByZXRyaWV2ZWQuIiB9CisgICAgICAgICAgICAgICAg XSwKKyAgICAgICAgICAgICAgICAiZGVzY3JpcHRpb24iOiAiUmVxdWVzdHMgdGhhdCBjaGlsZHJl biBvZiB0aGUgbm9kZSB3aXRoIGdpdmVuIGlkIGFyZSByZXR1cm5lZCB0byB0aGUgY2FsbGVyIGlu IGZvcm0gb2YgPGNvZGU+c2V0Q2hpbGROb2RlczwvY29kZT4gZXZlbnRzIHdoZXJlIG5vdCBvbmx5 IGltbWVkaWF0ZSBjaGlsZHJlbiBhcmUgcmV0cmlldmVkLCBidXQgYWxsIGNoaWxkcmVuIGRvd24g dG8gdGhlIHNwZWNpZmllZCBkZXB0aC4iCisgICAgICAgICAgICB9LAorICAgICAgICAgICAgewog ICAgICAgICAgICAgICAgICJuYW1lIjogInF1ZXJ5U2VsZWN0b3IiLAogICAgICAgICAgICAgICAg ICJwYXJhbWV0ZXJzIjogWwogICAgICAgICAgICAgICAgICAgICB7ICJuYW1lIjogIm5vZGVJZCIs ICIkcmVmIjogIk5vZGVJZCIsICJkZXNjcmlwdGlvbiI6ICJJZCBvZiB0aGUgbm9kZSB0byBxdWVy eSB1cG9uLiIgfSwKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0 b3JET01BZ2VudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yRE9NQWdl bnQuY3BwCmluZGV4IGZkNGY2ZTQ0ZGRiODAzYjM5N2NlZjBmOGQ2MjhhNDM3OTBlMGI5ZDAuLjlj MTc5ZTU2NzdkYTA0YWYxMDJiZjRhYThmYzFiODU4ODQ2MDIzYWQgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JET01BZ2VudC5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvaW5zcGVjdG9yL0luc3BlY3RvckRPTUFnZW50LmNwcApAQCAtNDMzLDcgKzQzMyw3IEBA IHZvaWQgSW5zcGVjdG9yRE9NQWdlbnQ6OmdldERvY3VtZW50KEVycm9yU3RyaW5nKiBlcnJvclN0 cmluZywgUmVmUHRyPFR5cGVCdWlsZGVyCiAgICAgcm9vdCA9IGJ1aWxkT2JqZWN0Rm9yTm9kZSht X2RvY3VtZW50LmdldCgpLCAyLCAmbV9kb2N1bWVudE5vZGVUb0lkTWFwKTsKIH0KIAotdm9pZCBJ bnNwZWN0b3JET01BZ2VudDo6cHVzaENoaWxkTm9kZXNUb0Zyb250ZW5kKGludCBub2RlSWQpCit2 b2lkIEluc3BlY3RvckRPTUFnZW50OjpwdXNoQ2hpbGROb2Rlc1RvRnJvbnRlbmQoaW50IG5vZGVJ ZCwgaW50IGRlcHRoKQogewogICAgIE5vZGUqIG5vZGUgPSBub2RlRm9ySWQobm9kZUlkKTsKICAg ICBpZiAoIW5vZGUgfHwgKG5vZGUtPm5vZGVUeXBlKCkgIT0gTm9kZTo6RUxFTUVOVF9OT0RFICYm IG5vZGUtPm5vZGVUeXBlKCkgIT0gTm9kZTo6RE9DVU1FTlRfTk9ERSAmJiBub2RlLT5ub2RlVHlw ZSgpICE9IE5vZGU6OkRPQ1VNRU5UX0ZSQUdNRU5UX05PREUpKQpAQCAtNDQyLDcgKzQ0Miw3IEBA IHZvaWQgSW5zcGVjdG9yRE9NQWdlbnQ6OnB1c2hDaGlsZE5vZGVzVG9Gcm9udGVuZChpbnQgbm9k ZUlkKQogICAgICAgICByZXR1cm47CiAKICAgICBOb2RlVG9JZE1hcCogbm9kZU1hcCA9IG1faWRU b05vZGVzTWFwLmdldChub2RlSWQpOwotICAgIFJlZlB0cjxUeXBlQnVpbGRlcjo6QXJyYXk8VHlw ZUJ1aWxkZXI6OkRPTTo6Tm9kZT4gPiBjaGlsZHJlbiA9IGJ1aWxkQXJyYXlGb3JDb250YWluZXJD aGlsZHJlbihub2RlLCAxLCBub2RlTWFwKTsKKyAgICBSZWZQdHI8VHlwZUJ1aWxkZXI6OkFycmF5 PFR5cGVCdWlsZGVyOjpET006Ok5vZGU+ID4gY2hpbGRyZW4gPSBidWlsZEFycmF5Rm9yQ29udGFp bmVyQ2hpbGRyZW4obm9kZSwgZGVwdGgsIG5vZGVNYXApOwogICAgIG1fZnJvbnRlbmQtPnNldENo aWxkTm9kZXMobm9kZUlkLCBjaGlsZHJlbi5yZWxlYXNlKCkpOwogfQogCkBAIC00ODMsNiArNDgz LDExIEBAIHZvaWQgSW5zcGVjdG9yRE9NQWdlbnQ6OnJlcXVlc3RDaGlsZE5vZGVzKEVycm9yU3Ry aW5nKiwgaW50IG5vZGVJZCkKICAgICBwdXNoQ2hpbGROb2Rlc1RvRnJvbnRlbmQobm9kZUlkKTsK IH0KIAordm9pZCBJbnNwZWN0b3JET01BZ2VudDo6cmVxdWVzdENoaWxkTm9kZXNXaXRoRGVwdGgo RXJyb3JTdHJpbmcqLCBpbnQgbm9kZUlkLCBpbnQgZGVwdGgpCit7CisgICAgcHVzaENoaWxkTm9k ZXNUb0Zyb250ZW5kKG5vZGVJZCwgZGVwdGgpOworfQorCiB2b2lkIEluc3BlY3RvckRPTUFnZW50 OjpxdWVyeVNlbGVjdG9yKEVycm9yU3RyaW5nKiBlcnJvclN0cmluZywgaW50IG5vZGVJZCwgY29u c3QgU3RyaW5nJiBzZWxlY3RvcnMsIGludCogZWxlbWVudElkKQogewogICAgICplbGVtZW50SWQg PSAwOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL0luc3BlY3RvckRPTUFn ZW50LmggYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yRE9NQWdlbnQuaAppbmRl eCBlMjNhODE4MGY0OTcwYjg5N2VmYWRjODczNDJhMDRmZTQ1YTFkZDU3Li5iNGU2MDk1ZjliZTU5 MjhiNTQ4NzM4ZWNmYTMzZWZhODIwYzIyZmNmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9p bnNwZWN0b3IvSW5zcGVjdG9yRE9NQWdlbnQuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0 b3IvSW5zcGVjdG9yRE9NQWdlbnQuaApAQCAtMTIzLDYgKzEyMyw3IEBAIHB1YmxpYzoKICAgICB2 aXJ0dWFsIHZvaWQgcXVlcnlTZWxlY3RvckFsbChFcnJvclN0cmluZyosIGludCBub2RlSWQsIGNv bnN0IFN0cmluZyYgc2VsZWN0b3JzLCBSZWZQdHI8VHlwZUJ1aWxkZXI6OkFycmF5PGludD4gPiYg cmVzdWx0KTsKICAgICB2aXJ0dWFsIHZvaWQgZ2V0RG9jdW1lbnQoRXJyb3JTdHJpbmcqLCBSZWZQ dHI8VHlwZUJ1aWxkZXI6OkRPTTo6Tm9kZT4mIHJvb3QpOwogICAgIHZpcnR1YWwgdm9pZCByZXF1 ZXN0Q2hpbGROb2RlcyhFcnJvclN0cmluZyosIGludCBub2RlSWQpOworICAgIHZpcnR1YWwgdm9p ZCByZXF1ZXN0Q2hpbGROb2Rlc1dpdGhEZXB0aChFcnJvclN0cmluZyosIGludCBub2RlSWQsIGlu dCBkZXB0aCk7CiAgICAgdmlydHVhbCB2b2lkIHNldEF0dHJpYnV0ZVZhbHVlKEVycm9yU3RyaW5n KiwgaW50IGVsZW1lbnRJZCwgY29uc3QgU3RyaW5nJiBuYW1lLCBjb25zdCBTdHJpbmcmIHZhbHVl KTsKICAgICB2aXJ0dWFsIHZvaWQgc2V0QXR0cmlidXRlc0FzVGV4dChFcnJvclN0cmluZyosIGlu dCBlbGVtZW50SWQsIGNvbnN0IFN0cmluZyYgdGV4dCwgY29uc3QgU3RyaW5nKiBuYW1lKTsKICAg ICB2aXJ0dWFsIHZvaWQgcmVtb3ZlQXR0cmlidXRlKEVycm9yU3RyaW5nKiwgaW50IGVsZW1lbnRJ ZCwgY29uc3QgU3RyaW5nJiBuYW1lKTsKQEAgLTIxOCw3ICsyMTksNyBAQCBwcml2YXRlOgogICAg IEVsZW1lbnQqIGFzc2VydEVkaXRhYmxlRWxlbWVudChFcnJvclN0cmluZyosIGludCBub2RlSWQp OwogCiAgICAgaW50IHB1c2hOb2RlUGF0aFRvRnJvbnRlbmQoTm9kZSopOwotICAgIHZvaWQgcHVz aENoaWxkTm9kZXNUb0Zyb250ZW5kKGludCBub2RlSWQpOworICAgIHZvaWQgcHVzaENoaWxkTm9k ZXNUb0Zyb250ZW5kKGludCBub2RlSWQsIGludCBkZXB0aCA9IDEpOwogCiAgICAgYm9vbCBoYXNC cmVha3BvaW50KE5vZGUqLCBpbnQgdHlwZSk7CiAgICAgdm9pZCB1cGRhdGVTdWJ0cmVlQnJlYWtw b2ludHMoTm9kZSogcm9vdCwgdWludDMyX3Qgcm9vdE1hc2ssIGJvb2wgdmFsdWUpOwo=