Bug 96730 - Web Inspector: do not use InspectorInstrumentation::hasFrontends() check when collecting stacks
Summary: Web Inspector: do not use InspectorInstrumentation::hasFrontends() check when...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Vsevolod Vlasov
URL:
Keywords:
Depends on: 97921
Blocks: 94433
  Show dependency tree / graph
 
Reported: 2012-09-13 23:21 PDT by Pavel Feldman
Modified: 2012-10-01 02:41 PDT (History)
20 users (show)

See Also:

Attachments
Patch (45.60 KB, patch)
2012-09-25 06:55 PDT, Pavel Feldman 		no flags Details | Formatted Diff | Diff
Patch (47.52 KB, patch)
2012-09-25 07:38 PDT, Pavel Feldman 		no flags Details | Formatted Diff | Diff
Patch (45.54 KB, patch)
2012-09-25 08:29 PDT, Pavel Feldman 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>