[webkit-reviews] review granted: [Bug 31142] DOM tree selection disappears upon page refresh : [Attachment 44145] [PATCH] Now with no 'animation'.

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 31142: DOM tree selection disappears upon page refresh
https://bugs.webkit.org/show_bug.cgi?id=31142

Attachment 44145: [PATCH] Now with no 'animation'.
https://bugs.webkit.org/attachment.cgi?id=44145&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +    for (size_t i = 0; i < pathTokens.size()  - 1; i += 2) {

Extra space before "- 1".


> +	   int childNumber = pathTokens[i].toInt(&success);
> +	   int nodesCount = innerChildNodeCount(node);

Can these be unsigned/size_t?


> +	   String childName = pathTokens[i + 1];

Move this down to where you need it.


> +	   for (int j = 0; child && j < childNumber; ++j)

j should be size_t.


>	   ~InspectorDOMAgent();
> +	   void reset();

Put a newline before reset so it isn't grouped with the destructor.


> +	   this.reset();
> +	   if (!inspectedRootDocument)
>	       return;

Put a newline after the reset call.


> +	      
InjectedScriptAccess.pushNodeByPathToFrontend(this._selectedPathOnReset,
selectLastSelectedNode.bind(this));

It seems odd to call a function called "pushNodeByPathToFrontend" from the
frontend! Should it just be "nodeByPath" and take a callback?