Web Replay: Playback position updates should be sent before the next event loop input...
authorburg@cs.washington.edu <burg@cs.washington.edu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Sep 2014 22:17:29 +0000 (22:17 +0000)
committerburg@cs.washington.edu <burg@cs.washington.edu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Sep 2014 22:17:29 +0000 (22:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=137162

Reviewed by Timothy Hatcher.

Source/WebCore:

To drive playback position updates in the Inspector UI, we send playbackHitPosition protocol
messages as the replay backend dispatches inputs. However, right now the semantics of that
message are muddy. The update is sent *after* the input at the offset is dispatched. This leads
to unexpected results if the debugger pauses while the input is being dispatched: the frontend
will only know about the previous (stale) playback position when the debugger pauses.

With this patch, the backend sends the playbackHitPosition(segmentOffset=n, inputOffset=m)
message when backend is about to dispatch input m, but has not yet begun to do so. Thus, any
subsequent page execution events (profiling, debugger pauses, etc) until the next
playbackHitPosition are caused by input m's being dispatched.

* inspector/protocol/Replay.json: Clarify the message's semantics.
* replay/ReplayController.cpp:
(WebCore::ReplayController::willDispatchInput):
(WebCore::ReplayController::didDispatchInput):

Source/WebInspectorUI:

Pausing playback from the UI was broken because of a typo. Fix this, and rename
stopPlayback to cancelPlayback.

* UserInterface/Controllers/ReplayManager.js:
(WebInspector.ReplayManager.prototype.switchSession.if):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174037 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/inspector/protocol/Replay.json
Source/WebCore/replay/ReplayController.cpp
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js

index 3cd267b..f85197a 100644 (file)
@@ -1,3 +1,26 @@
+2014-09-27  Brian J. Burg  <burg@cs.washington.edu>
+
+        Web Replay: Playback position updates should be sent before the next event loop input is dispatched
+        https://bugs.webkit.org/show_bug.cgi?id=137162
+
+        Reviewed by Timothy Hatcher.
+
+        To drive playback position updates in the Inspector UI, we send playbackHitPosition protocol
+        messages as the replay backend dispatches inputs. However, right now the semantics of that
+        message are muddy. The update is sent *after* the input at the offset is dispatched. This leads
+        to unexpected results if the debugger pauses while the input is being dispatched: the frontend
+        will only know about the previous (stale) playback position when the debugger pauses.
+
+        With this patch, the backend sends the playbackHitPosition(segmentOffset=n, inputOffset=m)
+        message when backend is about to dispatch input m, but has not yet begun to do so. Thus, any
+        subsequent page execution events (profiling, debugger pauses, etc) until the next
+        playbackHitPosition are caused by input m's being dispatched.
+
+        * inspector/protocol/Replay.json: Clarify the message's semantics.
+        * replay/ReplayController.cpp:
+        (WebCore::ReplayController::willDispatchInput):
+        (WebCore::ReplayController::didDispatchInput):
+
 2014-09-27  Benjamin Poulain  <bpoulain@apple.com>
 
         Chaining multiple :nth-child() does not work properly
index 7d8f565..5b5b00c 100644 (file)
         },
         {
             "name": "playbackHitPosition",
-            "description": "A position was reached during playback of the session.",
+            "description": "Playback within the session has progressed up to this position, and is about to replay the input at the specified offset.",
             "parameters": [
                 { "name": "position", "$ref": "ReplayPosition", "description": "The playback position that was hit." },
                 { "name": "timestamp", "type": "number", "description": "A timestamp for the event." }
index 8edceb2..96b1f14 100644 (file)
@@ -510,6 +510,9 @@ void ReplayController::willDispatchInput(const EventLoopInputBase&)
     ASSERT(m_segmentState == SegmentState::Dispatching);
 
     m_currentPosition.inputOffset++;
+
+    InspectorInstrumentation::playbackHitPosition(&m_page, m_currentPosition);
+
     if (m_currentPosition == m_targetPosition)
         pausePlayback();
 }
@@ -518,8 +521,6 @@ void ReplayController::didDispatchInput(const EventLoopInputBase&)
 {
     ASSERT(m_sessionState == SessionState::Replaying);
     ASSERT(m_segmentState == SegmentState::Dispatching);
-
-    InspectorInstrumentation::playbackHitPosition(&m_page, m_currentPosition);
 }
 
 void ReplayController::didDispatchFinalInput()
index 9b264d2..cdfd65f 100644 (file)
@@ -1,3 +1,16 @@
+2014-09-27  Brian J. Burg  <burg@cs.washington.edu>
+
+        Web Replay: Playback position updates should be sent before the next event loop input is dispatched
+        https://bugs.webkit.org/show_bug.cgi?id=137162
+
+        Reviewed by Timothy Hatcher.
+
+        Pausing playback from the UI was broken because of a typo. Fix this, and rename
+        stopPlayback to cancelPlayback.
+
+        * UserInterface/Controllers/ReplayManager.js:
+        (WebInspector.ReplayManager.prototype.switchSession.if):
+
 2014-09-26  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Automatic Inspection should continue once all breakpoints are loaded
index 46c0505..6994bc0 100644 (file)
@@ -213,7 +213,7 @@ WebInspector.ReplayManager.prototype = {
 
         if (this.sessionState === WebInspector.ReplayManager.SessionState.Replaying) {
             result = result.then(function() {
-                return WebInspector.replayManager.stopPlayback();
+                return WebInspector.replayManager.cancelPlayback();
             });
         }
 
@@ -244,7 +244,7 @@ WebInspector.ReplayManager.prototype = {
 
         if (this.sessionState === WebInspector.ReplayManager.SessionState.Replaying) {
             result = result.then(function() {
-                return WebInspector.replayManager.stopPlayback();
+                return WebInspector.replayManager.cancelPlayback();
             });
         }
 
@@ -289,7 +289,7 @@ WebInspector.ReplayManager.prototype = {
         if (this.sessionState === WebInspector.ReplayManager.SessionState.Inactive)
             return result; // Already stopped.
 
-        if (this.sessionState !== WebInspector.ReplayManager.SegmentState.Dispatching)
+        if (this.segmentState !== WebInspector.ReplayManager.SegmentState.Dispatching)
             return result; // Already stopped.
 
         result = result.then(function() {
@@ -307,7 +307,7 @@ WebInspector.ReplayManager.prototype = {
 
     // Pause playback and unload the current session segment as soon as possible.
     // Returns a promise that resolves when the current segment is unloaded.
-    stopPlayback: function() // --> ()
+    cancelPlayback: function() // --> ()
     {
         console.assert(this.sessionState !== WebInspector.ReplayManager.SessionState.Capturing, "Cannot stop playback while capturing.");
 
@@ -321,7 +321,7 @@ WebInspector.ReplayManager.prototype = {
                 console.assert(manager.sessionState === WebInspector.ReplayManager.SessionState.Replaying);
                 console.assert(manager.segmentState !== WebInspector.ReplayManager.SegmentState.Appending);
 
-                return ReplayAgent.stopPlayback();
+                return ReplayAgent.cancelPlayback();
             }).catch(function(error) {
                 console.error("Failed to stop playback: ", error);
                 throw error;