Bug 92739

Created attachment 155486 [details] Inspector Extension

Created attachment 155521 [details] Patch

Comment on attachment 155521 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155521&action=review > Source/WebCore/inspector/InspectorCSSAgent.cpp:540 > + int nodeId = m_domAgent->boundNodeId(static_cast<Node*>(document)); There is no need to cast document to Node, Document is a Node. > Source/WebCore/inspector/InspectorCSSAgent.cpp:541 > + if (!nodeId) I am not sure this is what you want. You only report creation / destruction of the flows when document node on the front-end is visible. So every time document node is received on the front-end (user expanded an <iframe> tag), you want to query for document's existing named flows and dispatch corresponding events. > Source/WebCore/inspector/InspectorCSSAgent.cpp:550 > + int nodeId = m_domAgent->boundNodeId(static_cast<Node*>(document)); ditto

Created attachment 156285 [details] Patch

Comment on attachment 156285 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156285&action=review Couple more nits, one of them depends on another bug Alexander is filing. > Source/WebCore/dom/WebKitNamedFlowCollection.cpp:83 > +#if ENABLE(INSPECTOR) You should not surround Instrumentation calls with #if ENABLE. > Source/WebCore/dom/WebKitNamedFlowCollection.cpp:101 > +#if ENABLE(INSPECTOR) ditto > Source/WebCore/inspector/InspectorCSSAgent.cpp:540 > + int nodeId = m_domAgent->pushNodePathToFrontend(document); pushChildNodesToFrontend implies there is a front-end. > Source/WebCore/inspector/InspectorCSSAgent.cpp:542 > + if (m_frontend) You should not make this check. In fact, you should not be getting into this method when there is no m_frontend. It is a bug in InspectorCSSAgent that needs to be addressed. I'll ask apavlov@ to handle it.

Comment on attachment 156285 [details] Patch r- per comments

Created attachment 156323 [details] Patch

Comment on attachment 156323 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156323&action=review Looks good, once issue below... > Source/WebCore/inspector/InspectorCSSAgent.cpp:540 > + m_frontend->namedFlowCreated(m_domAgent->pushNodePathToFrontend(document), name.string()); Sorry for not spotting this earlier. By design, protocol should not emit any events unless they are explicitly requested by the client. In your case, attaching to the backend will start populating DOM with nodes and issuing these events. So we need to figure out how to prevent that from happening. What we try to do in such cases is: - events are not generated unless getNamedFlowCollection is called for given document node - once getNamedFlowCollection was called at least once, namedFromCreated/Deleted events start flowing to the front-end That will make behaviour consistent: once you asked for the present state, you get update notifications, you would not need to call pushNodePathToFrontend anymore - boundNodeId would be sufficient.

Created attachment 156695 [details] Patch

Comment on attachment 156695 [details] Patch Clearing flags on attachment: 156695 Committed r124778: <http://trac.webkit.org/changeset/124778>

All reviewed patches have been landed. Closing bug.