Bug 92739 – Web Inspector: Protocol: Add "namedFlowCreated" and "namedFlowRemoved" events

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Andrei Poenaru]

Extends the protocol with "namedFlowCreated" and "namedFlowRemoved" events.

[Andrei Poenaru]

Created attachment 155486 [details]
Inspector Extension

[Andrei Poenaru]

Created attachment 155521 [details]
Patch

[Pavel Feldman]

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

[Andrei Poenaru]

Created attachment 156285 [details]
Patch

[Pavel Feldman]

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.

[Pavel Feldman]

Comment on attachment 156285 [details]
Patch

r- per comments

[Andrei Poenaru]

Created attachment 156323 [details]
Patch

[Pavel Feldman]

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.

[Andrei Poenaru]

Created attachment 156695 [details]
Patch

[WebKit Review Bot]

Comment on attachment 156695 [details]
Patch

Clearing flags on attachment: 156695

Committed r124778: <http://trac.webkit.org/changeset/124778>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.