Bug 190306 - Web Inspector: notify the frontend when a canvas has started recording via console.record
Summary: Web Inspector: notify the frontend when a canvas has started recording via co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 190305
Blocks: WebInspectorCanvasRecording WebInspectorCanvasTab
  Show dependency tree / graph
 
Reported: 2018-10-04 23:10 PDT by Devin Rousso
Modified: 2018-10-10 11:35 PDT (History)
10 users (show)

See Also:

Attachments
Patch (11.58 KB, patch)
2018-10-05 01:23 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Patch (11.57 KB, patch)
2018-10-05 01:29 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Patch (13.78 KB, patch)
2018-10-08 20:03 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-sierra (3.08 MB, application/zip)
2018-10-08 22:04 PDT, EWS Watchlist 		no flags Details
Patch (20.16 KB, patch)
2018-10-08 22:53 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) 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 Devin Rousso 2018-10-04 23:10:28 PDT 
This would prevent the unexpected state where `console.record` is called and then the frontend later requests a recording, resulting in an error.
Comment 1 Devin Rousso 2018-10-05 01:23:42 PDT 
Created attachment 351663 [details]
Patch
Comment 2 Devin Rousso 2018-10-05 01:29:57 PDT 
Created attachment 351664 [details]
Patch
Comment 3 Blaze Burg 2018-10-08 09:36:07 PDT 
Comment on attachment 351664 [details]
Patch

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

This change looks fine (excepting that it doesn't apply to TOT). Please rebase.

> Source/JavaScriptCore/inspector/protocol/Canvas.json:197
> +                { "name": "fromConsole", "type": "boolean" }

I'd prefer that this be "initiator" with enum { Frontend, ConsoleAPI }. Thinking ahead to the various ways console logs can be initiated, this seems more future-proof if we add other means to trigger this. I also just generically don't like adding boolean flags to the protocol.
Comment 4 Devin Rousso 2018-10-08 10:02:20 PDT 
Comment on attachment 351664 [details]
Patch

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

(In reply to Brian Burg from comment #3)
> This change looks fine (excepting that it doesn't apply to TOT). Please
> rebase.
It won't apply because it needs changes made in <https://webkit.org/b/190305> (which is blocking this one).

>> Source/JavaScriptCore/inspector/protocol/Canvas.json:197
>> +                { "name": "fromConsole", "type": "boolean" }
> 
> I'd prefer that this be "initiator" with enum { Frontend, ConsoleAPI }. Thinking ahead to the various ways console logs can be initiated, this seems more future-proof if we add other means to trigger this. I also just generically don't like adding boolean flags to the protocol.

Oooo I like it!
Comment 5 Devin Rousso 2018-10-08 20:03:51 PDT 
Created attachment 351849 [details]
Patch
Comment 6 EWS Watchlist 2018-10-08 22:04:56 PDT Comment hidden (obsolete)
Comment 7 EWS Watchlist 2018-10-08 22:04:57 PDT Comment hidden (obsolete)
Comment 8 Devin Rousso 2018-10-08 22:53:05 PDT 
Created attachment 351860 [details]
Patch
Comment 9 Blaze Burg 2018-10-10 11:02:27 PDT 
Comment on attachment 351860 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2018-10-10 11:34:56 PDT 
Comment on attachment 351860 [details]
Patch

Clearing flags on attachment: 351860

Committed r237010: <https://trac.webkit.org/changeset/237010>
Comment 11 WebKit Commit Bot 2018-10-10 11:34:58 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2018-10-10 11:35:24 PDT 
<rdar://problem/45167312>