Bug 151635

Summary: Web Inspector: control whether to collect and dump protocol messages using a WebInspector.Setting
Product: WebKit Reporter: Blaze Burg <bburg>
Component: Web InspectorAssignee: Blaze Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 137663, 147066    
Attachments:
Description Flags
Proposed Fix
none
Proposed Fix
timothy: review-
Proposed Fix
none
For Landing
none
Follow-up fix none

Blaze Burg
Reported 2015-11-27 17:28:28 PST
The first step towards inspector^2 tooling for analyzing protocol messages. See the meta bug for some big picture ideas.
Attachments
Proposed Fix (28.95 KB, patch)
2015-11-28 00:02 PST, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Proposed Fix (45.58 KB, patch)
2015-11-28 00:13 PST, Blaze Burg 		timothy: review-
DetailsFormatted DiffDiff
Proposed Fix (33.15 KB, patch)
2015-11-29 17:08 PST, Blaze Burg 		no flags
DetailsFormatted DiffDiff
For Landing (35.51 KB, patch)
2015-12-09 13:46 PST, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Follow-up fix (1.80 KB, patch)
2015-12-17 15:02 PST, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2015-11-27 17:28:41 PST
<rdar://problem/23679143>
Blaze Burg
Comment 2 2015-11-28 00:02:58 PST
Created attachment 266212 [details] Proposed Fix May not apply yet, due to being on top of 5 patches. I don't think it depends on them outright, though.
Blaze Burg
Comment 3 2015-11-28 00:13:10 PST
Created attachment 266213 [details] Proposed Fix
Timothy Hatcher
Comment 4 2015-11-28 13:35:50 PST
Comment on attachment 266213 [details] Proposed Fix Wrong patch.
Blaze Burg
Comment 5 2015-11-29 17:08:53 PST
Created attachment 266228 [details] Proposed Fix
Timothy Hatcher
Comment 6 2015-12-09 13:33:11 PST
Comment on attachment 266228 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=266228&action=review > Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:117 > + if (this.activeTracer) > + this.activeTracer.logStarted(); > + // If the custom tracer was removed and automatic tracing is enabled, > + // then create a new automatic tracer and install it in its place. > + else This style is confusing. The comment should move under the else and add {}.
Blaze Burg
Comment 7 2015-12-09 13:46:15 PST
Created attachment 267043 [details] For Landing
WebKit Commit Bot
Comment 8 2015-12-09 14:45:42 PST
Comment on attachment 267043 [details] For Landing Clearing flags on attachment: 267043 Committed r193870: <http://trac.webkit.org/changeset/193870>
WebKit Commit Bot
Comment 9 2015-12-09 14:45:46 PST
All reviewed patches have been landed. Closing bug.
Joseph Pecoraro
Comment 10 2015-12-14 19:08:25 PST
Comment on attachment 267043 [details] For Landing View in context: https://bugs.webkit.org/attachment.cgi?id=267043&action=review > Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:112 > + logWillHandleEvent(message) > + { > + console.assert(typeof message === "string", "Must stringify messages to avoid leaking all JSON protocol messages.") > + > + let entry = {type: "event", message}; > + this._processEntry(entry); > + } > + > + logDidHandleEvent(message, timings = null) > + { > + console.assert(typeof message === "string", "Must stringify messages to avoid leaking all JSON protocol messages.") > + > + let entry = {type: "event", message}; > + if (timings) > + entry.timings = Object.shallowCopy(timings); > + > + this._processEntry(entry); > + } This is processing the entry (logging to console) on both willHandle and didHandle. This means when dumping to the console its happening twice for each event (and response). What is the intended behavior here?
Blaze Burg
Comment 11 2015-12-15 09:05:57 PST
Comment on attachment 267043 [details] For Landing View in context: https://bugs.webkit.org/attachment.cgi?id=267043&action=review >> Source/WebInspectorUI/UserInterface/Protocol/LoggingProtocolTracer.js:112 >> + } > > This is processing the entry (logging to console) on both willHandle and didHandle. This means when dumping to the console its happening twice for each event (and response). What is the intended behavior here? The prior behavior was to log twice only if time-stats are enabled. Now it always logs twice. We could change the outer else if to be (this._dumpMessagesToConsole && !entry.timings).
Blaze Burg
Comment 12 2015-12-17 15:02:09 PST
Reopening to attach new patch.
Blaze Burg
Comment 13 2015-12-17 15:02:11 PST
Created attachment 267585 [details] Follow-up fix
Joseph Pecoraro
Comment 14 2015-12-17 15:05:32 PST
Comment on attachment 267585 [details] Follow-up fix r=me
WebKit Commit Bot
Comment 15 2015-12-17 15:54:18 PST
Comment on attachment 267585 [details] Follow-up fix Clearing flags on attachment: 267585 Committed r194244: <http://trac.webkit.org/changeset/194244>
WebKit Commit Bot
Comment 16 2015-12-17 15:54:22 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.