Bug 135249

Summary: Web Inspector: convert ReplayManager to a promise-based API
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Brian Burg <burg>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 135608    
Attachments:
Description Flags
Patch timothy: review+

Description Brian Burg 2014-07-24 12:37:50 PDT
This allows the client code to handle fewer replay state cases because the replay manager can asynchronously "make it so". It also makes capture/replay more robust, because the manager can asynchronously disable breakpoints, continue the debugger, etc. before carrying out commands.

For example, if a startCapturing() command is issued during replay, the manager can construct a promise chain to stop replaying, unload the current segment, and start capturing. Without an async promise API, each client would have to manually get the replay engine to an acceptable state.
Comment 1 Radar WebKit Bug Importer 2014-07-24 12:38:27 PDT
<rdar://problem/17798570>
Comment 2 Brian Burg 2014-07-24 13:03:49 PDT
Created attachment 235448 [details]
Patch
Comment 3 Timothy Hatcher 2014-08-05 11:06:40 PDT
Comment on attachment 235448 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:231
> +    switchSession: function(sessionId) { // --> ()

What is the "// --> ()" for?

> Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:253
> +            })
> +            .then(function ensureSessionDataIsLoaded(session) {

I like these on the same line.

> Source/WebInspectorUI/UserInterface/Views/ReplayDashboardView.js:124
> +        }).then(function() {

Yay! Same line.
Comment 4 Brian Burg 2014-08-05 12:47:22 PDT
Comment on attachment 235448 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:231
>> +    switchSession: function(sessionId) { // --> ()
> 
> What is the "// --> ()" for?

This is my (crappy) syntax for documenting which functions return a promise, and what the fulfilled promise value is (if any). Contrast to the getSegment() function.

>> Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:253
>> +            .then(function ensureSessionDataIsLoaded(session) {
> 
> I like these on the same line.

OK
Comment 5 Brian Burg 2014-08-06 11:03:29 PDT
If it's okay with you, I'm going to land this without the UI changes, since those aren't reviewed yet.
Comment 6 Brian Burg 2014-08-06 11:30:17 PDT
Committed r172161: <http://trac.webkit.org/changeset/172161>