Bug 96730

Summary: Web Inspector: do not use InspectorInstrumentation::hasFrontends() check when collecting stacks
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Vsevolod Vlasov <vsevik>
Status: RESOLVED FIXED    
Severity: Normal CC: aandrey, abarth, apavlov, bweinstein, dglazkov, gtk-ews, gustavo, haraken, japhet, joepeck, keishi, loislo, mkwst, pfeldman, pmuellr, rik, timothy, webkit.review.bot, xan.lopez, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 97921    
Bug Blocks: 94433    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Pavel Feldman 2012-09-13 23:21:46 PDT 
We should instead check for the corresponding agents to be enabled (network, timeline).
Comment 1 Pavel Feldman 2012-09-25 06:55:21 PDT 
Created attachment 165604 [details]
Patch
Comment 2 kov's GTK+ EWS bot 2012-09-25 07:06:21 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14032091
Comment 3 WebKit Review Bot 2012-09-25 07:07:17 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14006511
Comment 4 Early Warning System Bot 2012-09-25 07:09:12 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14035066
Comment 5 Gyuyoung Kim 2012-09-25 07:10:51 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14021123
Comment 6 Early Warning System Bot 2012-09-25 07:11:10 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14006512
Comment 7 Build Bot 2012-09-25 07:16:51 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14036037
Comment 8 Peter Beverloo (cr-android ews) 2012-09-25 07:16:55 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14035068
Comment 9 Build Bot 2012-09-25 07:18:27 PDT 
Comment on attachment 165604 [details]
Patch

Attachment 165604 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14027115
Comment 10 Pavel Feldman 2012-09-25 07:38:06 PDT 
Created attachment 165612 [details]
Patch
Comment 11 Andrey Adaikin 2012-09-25 08:12:56 PDT 
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()
Comment 12 Andrey Adaikin 2012-09-25 08:13:43 PDT 
(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
Comment 13 Pavel Feldman 2012-09-25 08:29:13 PDT 
Created attachment 165617 [details]
Patch
Comment 14 WebKit Review Bot 2012-09-27 04:18:07 PDT 
Comment on attachment 165617 [details]
Patch

Clearing flags on attachment: 165617

Committed r129751: <http://trac.webkit.org/changeset/129751>
Comment 15 WebKit Review Bot 2012-09-27 04:18:12 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 16 Adam Barth 2012-09-27 09:58:03 PDT 
Mike, this patch might effect your eval stack collection patch.
Comment 17 WebKit Review Bot 2012-09-28 09:41:57 PDT 
Re-opened since this is blocked by bug 97921
Comment 18 Vsevolod Vlasov 2012-09-28 09:49:12 PDT 
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)
Comment 19 Pavel Feldman 2012-10-01 02:41:11 PDT 
Committed r130021: <http://trac.webkit.org/changeset/130021>