Bug 135249 – Web Inspector: convert ReplayManager to a promise-based API

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Brian Burg]

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.

[Radar WebKit Bug Importer]

<rdar://problem/17798570>

[Brian Burg]

Created attachment 235448 [details]
Patch

[Timothy Hatcher]

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.

[Brian Burg]

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

[Brian Burg]

If it's okay with you, I'm going to land this without the UI changes, since those aren't reviewed yet.

[Brian Burg]

Committed r172161: <http://trac.webkit.org/changeset/172161>