Bug 135212

Summary: Web Inspector: ReplayManager shouldn't assume replay status when the inspector is opened
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Brian Burg <burg>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 135225    
Attachments:
Description Flags
Patch timothy: review+

Brian Burg
Reported 2014-07-23 14:33:54 PDT
The current implementation blindly assumes that no capturing or replaying has happened yet when the ReplayManager is being constructed. This breaks replay when the inspector is closed and then later reopened.
Attachments
Patch (21.43 KB, patch)
2014-07-23 15:06 PDT, Brian Burg 		timothy: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-07-23 14:34:28 PDT
<rdar://problem/17785105>
Brian Burg
Comment 2 2014-07-23 15:06:36 PDT
Created attachment 235384 [details] Patch
WebKit Commit Bot
Comment 3 2014-07-23 15:09:27 PDT
Attachment 235384 [details] did not pass style-queue: ERROR: Source/WebCore/inspector/InspectorReplayAgent.h:107: The parameter name "sessionState" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/inspector/InspectorReplayAgent.h:107: The parameter name "segmentState" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Timothy Hatcher
Comment 4 2014-08-05 11:44:37 PDT
Comment on attachment 235384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235384&action=review > Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:55 > + this._initializationPromise = ReplayAgent.currentReplayState.promise() No need for .promise() now. > Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:68 > + }) > + .then(function() { Same line? > Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:69 > + return ReplayAgent.getAvailableSessions.promise(); No .promise(). > Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:210 > + if (!this._initialized) > + return this.waitUntilInitialized().then(this.captureStarted.bind(this)); Very cool! Too bad this can't be more automagic of all of these.
Brian Burg
Comment 5 2014-08-05 14:57:13 PDT
Committed r172087: <http://trac.webkit.org/changeset/172087>
Brian Burg
Comment 6 2014-08-05 14:59:02 PDT
Comment on attachment 235384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235384&action=review >> Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:55 >> + this._initializationPromise = ReplayAgent.currentReplayState.promise() > > No need for .promise() now. That's not landed yet. >> Source/WebInspectorUI/UserInterface/Controllers/ReplayManager.js:210 >> + return this.waitUntilInitialized().then(this.captureStarted.bind(this)); > > Very cool! Too bad this can't be more automagic of all of these. It would require some sort of property proxying thing, which will be just as slow and a bunch of work.. :(
Note You need to log in before you can comment on or make changes to this bug.