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

Pavel Feldman
Reported 2012-09-13 23:21:46 PDT
We should instead check for the corresponding agents to be enabled (network, timeline).
Attachments
Patch (45.60 KB, patch)
2012-09-25 06:55 PDT, Pavel Feldman 		no flags
DetailsFormatted DiffDiff
Patch (47.52 KB, patch)
2012-09-25 07:38 PDT, Pavel Feldman 		no flags
DetailsFormatted DiffDiff
Patch (45.54 KB, patch)
2012-09-25 08:29 PDT, Pavel Feldman 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Pavel Feldman
Comment 1 2012-09-25 06:55:21 PDT
Created attachment 165604 [details] Patch
kov's GTK+ EWS bot
Comment 2 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
WebKit Review Bot
Comment 3 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
Early Warning System Bot
Comment 4 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
Gyuyoung Kim
Comment 5 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
Early Warning System Bot
Comment 6 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
Build Bot
Comment 7 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
Peter Beverloo (cr-android ews)
Comment 8 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
Build Bot
Comment 9 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
Pavel Feldman
Comment 10 2012-09-25 07:38:06 PDT
Created attachment 165612 [details] Patch
Andrey Adaikin
Comment 11 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()
Andrey Adaikin
Comment 12 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
Pavel Feldman
Comment 13 2012-09-25 08:29:13 PDT
Created attachment 165617 [details] Patch
WebKit Review Bot
Comment 14 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>
WebKit Review Bot
Comment 15 2012-09-27 04:18:12 PDT
All reviewed patches have been landed. Closing bug.
Adam Barth
Comment 16 2012-09-27 09:58:03 PDT
Mike, this patch might effect your eval stack collection patch.
WebKit Review Bot
Comment 17 2012-09-28 09:41:57 PDT
Re-opened since this is blocked by bug 97921
Vsevolod Vlasov
Comment 18 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)
Pavel Feldman
Comment 19 2012-10-01 02:41:11 PDT
Committed r130021: <http://trac.webkit.org/changeset/130021>
Note You need to log in before you can comment on or make changes to this bug.