Bug 96730 – Web Inspector: do not use InspectorInstrumentation::hasFrontends() check when collecting stacks

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

[Pavel Feldman]

We should instead check for the corresponding agents to be enabled (network, timeline).

[Pavel Feldman]

Created attachment 165604 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14032091

[WebKit Review Bot]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14006511

[Early Warning System Bot]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14035066

[Gyuyoung Kim]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14021123

[Early Warning System Bot]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14006512

[Build Bot]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14036037

[Peter Beverloo (cr-android ews)]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14035068

[Build Bot]

Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14027115

[Pavel Feldman]

Created attachment 165612 [details]
Patch

[Andrey Adaikin]

Comment on attachment 165612 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=165612&action=review

> Source/WebCore/inspector/InspectorCanvasAgent.h:67
> +    bool enabled() { return m_enabled; }

plz remove this. CanvasAgent is enabled if it has been set in the InstrumentingAgents. e.g. the canvasAgentEnabled() check resemble to the timelineAgentEnabled(), rather than consoleAgentEnabled()

[Andrey Adaikin]

(In reply to comment #11)
> (From update of attachment 165612 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165612&action=review
> 
> > Source/WebCore/inspector/InspectorCanvasAgent.h:67
> > +    bool enabled() { return m_enabled; }
> 
> plz remove this. CanvasAgent is enabled if it has been set in the InstrumentingAgents. e.g. the canvasAgentEnabled() check resemble to the timelineAgentEnabled(), rather than consoleAgentEnabled()

*resemble -> should resemble

[Pavel Feldman]

Created attachment 165617 [details]
Patch

[WebKit Review Bot]

Comment on attachment 165617 [details]
Patch

Clearing flags on attachment: 165617

Committed r129751: <http://trac.webkit.org/changeset/129751>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

Mike, this patch might effect your eval stack collection patch.

[WebKit Review Bot]

Re-opened since this is blocked by bug 97921

[Vsevolod Vlasov]

This patched weas rolled out due to crashes, see http://code.google.com/p/chromium/issues/detail?id=152947

I think this might have been caused by the lack of the non-null check of instrumentingAgentsForContext(scriptExecutionContext) in 

    return instrumentingAgentsForContext(scriptExecutionContext)->inspectorTimelineAgent();

(InspectorInstrumentation.cpp:1165)

[Pavel Feldman]

Committed r130021: <http://trac.webkit.org/changeset/130021>