[webkit-reviews] review denied: [Bug 92739] Web Inspector: Protocol: Add "namedFlowCreated" and "namedFlowRemoved" events : [Attachment 156323] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrei Poenaru
<poenaru at adobe.com>'s request for review:
Bug 92739: Web Inspector: Protocol: Add "namedFlowCreated" and
"namedFlowRemoved" events
https://bugs.webkit.org/show_bug.cgi?id=92739

Attachment 156323: Patch
https://bugs.webkit.org/attachment.cgi?id=156323&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
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.