Web Inspector: dispatch inspectNode to frontend without injected script. This will allow to inspect nodes while debugging.
Created attachment 195293 [details] Patch
Comment on attachment 195293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195293&action=review > Source/WebCore/inspector/Inspector.json:1992 > + "name": "shouldInspectNode", You don't want them on DOM domain, you also want them to be hidden. > Source/WebCore/inspector/InspectorDOMAgent.cpp:1031 > + int nodeId = pushNodePathToFrontend(m_nodeToFocus.get()); It might be too early to push the node - document might not be requested yet.
This seems overly complex — 2 methods and 1 event. What problem does this solve? Why is this needed? "Why" should always be answered in the bug and/or change log.
(In reply to comment #3) > This seems overly complex — 2 methods and 1 event. > > What problem does this solve? Why is this needed? "Why" should always be answered in the bug and/or change log. As written in the bug description, this will allow to inspect nodes while debugging. Otherwise, security checks disallow to evaluate anything in injected script, if debugger stopped in another context (i.e. in iframe). As for 2 additional methods, ChangeLog contains the explanation.
Comment on attachment 195293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195293&action=review >> Source/WebCore/inspector/Inspector.json:1992 >> + "name": "shouldInspectNode", > > You don't want them on DOM domain, you also want them to be hidden. Which domain then? Inspector? Is it OK to use them from InspectorDOMAgent class then (I mean, it will introduce dependence InspectorDOMAgent <-- InspectorFrontend::Inspector) ? >> Source/WebCore/inspector/InspectorDOMAgent.cpp:1031 >> + int nodeId = pushNodePathToFrontend(m_nodeToFocus.get()); > > It might be too early to push the node - document might not be requested yet. If so, this method will not do anything, and frontend will request nodeToFocus via getInspectedNode instead (which will happen after pushing document). See explanation in ChangeLog.
Sorry I missed the bug description. Reading through it all again it makes sense, though still more complex than I would like.
Created attachment 195969 [details] Patch
Created attachment 196109 [details] Patch
Separate event in DOM domain is the right thing here. This also solves the problem with inspecting in debug mode, since we don't go into injected script.
Comment on attachment 196109 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196109&action=review > Source/WebCore/inspector/Inspector.json:2074 > + "name": "nodeShouldBeInspected", inspectNodeRequested > Source/WebCore/inspector/InspectorDOMAgent.cpp:1033 > + if (m_searchingForNode) { Lets remove this "focusNode" method - it is misleading. > Source/WebCore/inspector/front-end/DOMAgent.js:1050 > + WebInspector.updateFocusedNode(nodeId); DOMAgent is SDK, WebInspector is UI, no dependency is allowed.
Created attachment 196145 [details] Patch
Comment on attachment 196109 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196109&action=review >> Source/WebCore/inspector/Inspector.json:2074 >> + "name": "nodeShouldBeInspected", > > inspectNodeRequested Done. >> Source/WebCore/inspector/InspectorDOMAgent.cpp:1033 >> + if (m_searchingForNode) { > > Lets remove this "focusNode" method - it is misleading. Done. Code separated between InspectorController::inspect and InspectorDOMAgent::inspect. >> Source/WebCore/inspector/front-end/DOMAgent.js:1050 >> + WebInspector.updateFocusedNode(nodeId); > > DOMAgent is SDK, WebInspector is UI, no dependency is allowed. Fixed.
Comment on attachment 196145 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196145&action=review > Source/WebCore/inspector/InspectorController.cpp:346 > + InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(mainWorldScriptState(frame)); if (node->nodeType() != Node::ELEMENT_NODE && node->nodeType() != Node::DOCUMENT_NODE) is now missing here.
Comment on attachment 196145 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196145&action=review >> Source/WebCore/inspector/InspectorController.cpp:346 >> + InjectedScript injectedScript = m_injectedScriptManager->injectedScriptFor(mainWorldScriptState(frame)); > > if (node->nodeType() != Node::ELEMENT_NODE && node->nodeType() != Node::DOCUMENT_NODE) is now missing here. Nice catch! Thanks. Fixed.
Created attachment 196325 [details] Patch