135311 2014-07-25 16:19:57 -0700 Web Inspector: shown() called on a content view when stepping over an instruction in the debugger 2014-08-05 11:30:09 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 saam webkit-unassigned burg commit-queue graouts joepeck timothy webkit-bug-importer oldest_to_newest 1024951 0 saam 2014-07-25 16:19:57 -0700 shown() is called on a ContentView every time the debugger steps over the instruction. Do we want to keep these semantics? If so, I'd argue it's apt to rename this function because it doesn't necessarily match when the screen is actually 'shown'. Or maybe, we should prevent screens that are already visible from having 'shown' called on them. The origin of the shown() is probably caused by an event from DebuggerManager every time it steps over a function. 1024952 1 webkit-bug-importer 2014-07-25 16:20:11 -0700 <rdar://problem/17817144> 1025017 2 timothy 2014-07-26 09:12:33 -0700 It wasn't intended to be called multiple times. 1025514 3 saam 2014-07-29 10:56:19 -0700 It seems like the main problem is that when stepping over an instruction, the debugger on the JSC side skips to the next instruction, and then emits the paused event. When the Inspector sees this pause event, it adds a new backForwardEntry and shows that new backForwardEntry, even if the content view it shows is for the same Resource. So, if stepping over every instruction in a JS file, there will be backForwardEntries for every instruction that's been stepped over. So clicking the back button at the top of the content browser will just show older content views that are really just the same file that we've been stepping over instructions within that file. The problem, then, doesn't seem to be that shown() is called multiple times on the same content view (because new content views for the same resource are repeatedly created), but rather, that we create new content views for every instruction we step over. This also seems bad from the user experience point of view of stepping over ten instructions, and then wanting to click back to go back to the previous JS file, and having to click back 10 times just to skip over these 10 content views that were created for each pause event. I'm not sure what the correct solution for this is, but I think the way it is set up now isn't the correct architecture. What do you all think? 1025517 4 timothy 2014-07-29 11:05:18 -0700 The back/forward list is not just resources (views) but also scroll positions and other cookie data. So the back button should be jumping around in the view to previous scroll positions. This is how Xcode behaves. shown() should still probably not be called if the view does not change. 1025518 5 timothy 2014-07-29 11:06:38 -0700 We probably should not push new back forward entries for each pause/resume if the scroll position didn't change. 1025519 6 saam 2014-07-29 11:09:21 -0700 (In reply to comment #5) > We probably should not push new back forward entries for each pause/resume if the scroll position didn't change. Yeah, that seems reasonable to me. I think saving them for each pause/resume is overkill when stepping over instructions. 1025523 7 timothy 2014-07-29 11:12:33 -0700 It looks like this code is at fault: if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) return currentEntry.contentView; It only considers the cookie and view when testing if a new entry needs to be pushed. It should also compare scroll positions. 1025524 8 joepeck 2014-07-29 11:14:20 -0700 (In reply to comment #3) > The problem, then, doesn't seem to be that shown() is called multiple times on the same content view (because new content views for the same resource are repeatedly created), but rather, that we create new content views for every instruction we step over. Do we really create a new ContentView? > This also seems bad from the user experience point of view of stepping over ten instructions, and then wanting to click back to go back to the previous JS file, and having to click back 10 times just to skip over these 10 content views that were created for each pause event. That sounds unintentional. I think the back forward list should be for specific user events. 1. Showing a ContentView, by clicking a Script/Resource Tree Element in the source sidebar 2. Jumping to a location by clicking a Breakpoint Tree Element in the debugger sidebar 3. Jumping to a location by clicking a CallFrame Tree Element in the debugger sidebar 4. Jumping to a location by clicking a (source:line:column) link anywhere in the UI 5. Hiding a ContentView, this may want to add a new entry or replace the current entry I think stepping in the debugger it would be unexpected that that adds a new back/forward entry other than the possibility of normal navigation with (1) or (5). 1025525 9 joepeck 2014-07-29 11:15:00 -0700 (In reply to comment #8) > the source sidebar Resource* sidebar 1025526 10 saam 2014-07-29 11:16:13 -0700 (In reply to comment #7) > It looks like this code is at fault: > > if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) > return currentEntry.contentView; > > > It only considers the cookie and view when testing if a new entry needs to be pushed. It should also compare scroll positions. Okay, I'll look into it. I was actually looking at that code path earlier, and it actually does what it's supposed two thirds of the time. It seems like that code path is travelled down 3 times for each step-over, and on one of the three paths when stepping over, that test fails. 1025527 11 timothy 2014-07-29 11:17:22 -0700 (In reply to comment #10) > (In reply to comment #7) > > It looks like this code is at fault: > > > > if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) > > return currentEntry.contentView; > > > > > > It only considers the cookie and view when testing if a new entry needs to be pushed. It should also compare scroll positions. > > Okay, I'll look into it. I was actually looking at that code path earlier, and it actually does what it's supposed two thirds of the time. It seems like that code path is travelled down 3 times for each step-over, and on one of the three paths when stepping over, that test fails. Three times sounds like two too many. 1025528 12 saam 2014-07-29 11:19:57 -0700 (In reply to comment #8) > > Do we really create a new ContentView? > I'm not 100% sure on this. I think the more accurate statement is new BackForwardEntries are created. > > That sounds unintentional. I think the back forward list should be for specific user events. > > 1. Showing a ContentView, by clicking a Script/Resource Tree Element in the source sidebar > 2. Jumping to a location by clicking a Breakpoint Tree Element in the debugger sidebar > 3. Jumping to a location by clicking a CallFrame Tree Element in the debugger sidebar > 4. Jumping to a location by clicking a (source:line:column) link anywhere in the UI > 5. Hiding a ContentView, this may want to add a new entry or replace the current entry > > I think stepping in the debugger it would be unexpected that that adds a new back/forward entry other than the possibility of normal navigation with (1) or (5). Yeah that seems reasonable. I would also add that a new BackForwardEntry should be added when a debugger paused event causes you to move to a completely new location within the same file. 1025529 13 saam 2014-07-29 11:21:56 -0700 (In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #7) > > > It looks like this code is at fault: > > > > > > if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) > > > return currentEntry.contentView; > > > > > > > > > It only considers the cookie and view when testing if a new entry needs to be pushed. It should also compare scroll positions. > > > > Okay, I'll look into it. I was actually looking at that code path earlier, and it actually does what it's supposed two thirds of the time. It seems like that code path is travelled down 3 times for each step-over, and on one of the three paths when stepping over, that test fails. > > Three times sounds like two too many. Yeah this is a stack trace after stepping over an instruction: file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:138:22: CONSOLE TRACE 0: showContentView(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:138:22) 1: showContentViewForRepresentedObject(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentBrowser.js:158:58) 2: showSourceCode(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceSidebarPanel.js:181:72) 3: showSourceCodeLocation(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceSidebarPanel.js:186:28) 4: _treeElementSelected(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/DebuggerSidebarPanel.js:525:69) 5: (unknown)([native code]) 6: select(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/TreeOutline.js:976:34) 7: _debuggerCallFramesDidChange(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/DebuggerSidebarPanel.js:395:39) 8: dispatch(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Base/Object.js:180:55) 9: dispatchEventToListeners(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Base/Object.js:187:17) 10: debuggerDidPause(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Controllers/DebuggerManager.js:379:38) 11: paused(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/DebuggerObserver.js:58:54) 12: dispatchEvent(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:309:42) 13: _dispatchEvent(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:215:32) 14: dispatch(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:89:32) 15: dispatchNextQueuedMessageFromBackend(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/MessageDispatcher.js:31:34) 16: (unknown)([native code]) 17: global code(file:///Volumes/Data/test.js:2:1) file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:138:22: CONSOLE TRACE 0: showContentView(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:138:22) 1: _showContentViewForIdentifier(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceClusterContentView.js:236:57) 2: restoreFromCookie(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceClusterContentView.js:147:61) 3: _restoreFromCookie(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Models/BackForwardEntry.js:77:43) 4: prepareToShow(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Models/BackForwardEntry.js:57:32) 5: _showEntry(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:402:28) 6: showBackForwardEntryForIndex(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:203:24) 7: showContentView(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:180:42) 8: showContentViewForRepresentedObject(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentBrowser.js:158:58) 9: showSourceCode(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceSidebarPanel.js:181:72) 10: showSourceCodeLocation(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceSidebarPanel.js:186:28) 11: _treeElementSelected(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/DebuggerSidebarPanel.js:525:69) 12: (unknown)([native code]) 13: select(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/TreeOutline.js:976:34) 14: _debuggerCallFramesDidChange(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/DebuggerSidebarPanel.js:395:39) 15: dispatch(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Base/Object.js:180:55) 16: dispatchEventToListeners(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Base/Object.js:187:17) 17: debuggerDidPause(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Controllers/DebuggerManager.js:379:38) 18: paused(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/DebuggerObserver.js:58:54) 19: dispatchEvent(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:309:42) 20: _dispatchEvent(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:215:32) 21: dispatch(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:89:32) 22: dispatchNextQueuedMessageFromBackend(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/MessageDispatcher.js:31:34) 23: (unknown)([native code]) 24: global code(file:///Volumes/Data/test.js:2:1) file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:138:22: CONSOLE TRACE 0: showContentView(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:138:22) 1: _showContentViewForIdentifier(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceClusterContentView.js:236:57) 2: shown(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceClusterContentView.js:130:43) 3: prepareToShow(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Models/BackForwardEntry.js:60:31) 4: _showEntry(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:402:28) 5: showBackForwardEntryForIndex(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:203:24) 6: showContentView(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentViewContainer.js:180:42) 7: showContentViewForRepresentedObject(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentBrowser.js:158:58) 8: showSourceCode(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceSidebarPanel.js:181:72) 9: showSourceCodeLocation(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ResourceSidebarPanel.js:186:28) 10: _treeElementSelected(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/DebuggerSidebarPanel.js:525:69) 11: (unknown)([native code]) 12: select(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/TreeOutline.js:976:34) 13: _debuggerCallFramesDidChange(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/DebuggerSidebarPanel.js:395:39) 14: dispatch(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Base/Object.js:180:55) 15: dispatchEventToListeners(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Base/Object.js:187:17) 16: debuggerDidPause(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Controllers/DebuggerManager.js:379:38) 17: paused(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/DebuggerObserver.js:58:54) 18: dispatchEvent(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:309:42) 19: _dispatchEvent(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:215:32) 20: dispatch(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/InspectorBackend.js:89:32) 21: dispatchNextQueuedMessageFromBackend(file:///Volumes/Data/WK/OpenSource/WebKitBuild/Release/WebInspectorUI.framework/Resources/Protocol/MessageDispatcher.js:31:34) 22: (unknown)([native code]) 23: global code(file:///Volumes/Data/test.js:2:1) 1025535 14 timothy 2014-07-29 11:50:25 -0700 (In reply to comment #13) > (In reply to comment #11) > > (In reply to comment #10) > > > (In reply to comment #7) > > > > It looks like this code is at fault: > > > > > > > > if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) > > > > return currentEntry.contentView; > > > > > > > > > > > > It only considers the cookie and view when testing if a new entry needs to be pushed. It should also compare scroll positions. > > > > > > Okay, I'll look into it. I was actually looking at that code path earlier, and it actually does what it's supposed two thirds of the time. It seems like that code path is travelled down 3 times for each step-over, and on one of the three paths when stepping over, that test fails. > > > > Three times sounds like two too many. > > Yeah this is a stack trace after stepping over an instruction: > > [snip] So this is a case of ResourceClusterContentView showing a sub-view, so this is likely correct. I think we should add an isEqual function to BackForwardEntry that also looks at _contentView, _cookie and _scrollPositions. 1025541 15 timothy 2014-07-29 12:29:01 -0700 (In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #11) > > > (In reply to comment #10) > > > > (In reply to comment #7) > > > > > It looks like this code is at fault: > > > > > > > > > > if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(provisionalEntry.cookie, currentEntry.cookie)) > > > > > return currentEntry.contentView; > > > > > > > > > > > > > > > It only considers the cookie and view when testing if a new entry needs to be pushed. It should also compare scroll positions. > > > > > > > > Okay, I'll look into it. I was actually looking at that code path earlier, and it actually does what it's supposed two thirds of the time. It seems like that code path is travelled down 3 times for each step-over, and on one of the three paths when stepping over, that test fails. > > > > > > Three times sounds like two too many. > > > > Yeah this is a stack trace after stepping over an instruction: > > > > [snip] > > So this is a case of ResourceClusterContentView showing a sub-view, so this is likely correct. > > I think we should add an isEqual function to BackForwardEntry that also looks at _contentView, _cookie and _scrollPositions. While that would fix some other cases, it will not fix views that use CodeMirror since _scrollPositions is not used for those. So there is another bug with CodeMirror views not saving scroll positions. CodeMirror views do save the last "jump" location to the cookie, which will be used for this with a showSourceCodeLocation call. And the cookie is already compared. So i think the behavior is correct as-is, where each jump to a new line in the same view pushes a new b/f entry. Not really the best behavior. Still shown() should not be called if it is already showing the view. 1025625 16 235724 saam 2014-07-29 19:28:15 -0700 Created attachment 235724 Idea for patch What do you guys think of this patch? It solves the issues I'm having with shown() being called multiple times. But maybe calling _restoreFromCookie() is an abstraction violation. But this is the essence of what a patch to fix this would be, I think. 1025646 17 235724 timothy 2014-07-29 20:55:00 -0700 Comment on attachment 235724 Idea for patch View in context: https://bugs.webkit.org/attachment.cgi?id=235724&action=review Looks right! > Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:205 > + currentEntry._restoreFromCookie(); Yeah, restoreFromCookie should drop the underscore and be a public method if this needs to call it. 1025674 18 235724 timothy 2014-07-29 23:31:59 -0700 Comment on attachment 235724 Idea for patch Actually it might be best to make a helper to do this. There are a couple other places where we do the _hideEntry(previousEntry) / _showEntry(currentEntry) dance. 1025736 19 saam 2014-07-30 10:46:42 -0700 (In reply to comment #17) > (From update of attachment 235724 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235724&action=review > > Looks right! > > > Source/WebInspectorUI/UserInterface/Views/ContentViewContainer.js:205 > > + currentEntry._restoreFromCookie(); > > Yeah, restoreFromCookie should drop the underscore and be a public method if this needs to call it. Okay. I'll probably end up doing that. Is it the intention of restoreFromCookie to be called more than once? Or maybe should there be another interface to updating the lineNumber info. 1025737 20 saam 2014-07-30 10:47:07 -0700 (In reply to comment #18) > (From update of attachment 235724 [details]) > Actually it might be best to make a helper to do this. There are a couple other places where we do the _hideEntry(previousEntry) / _showEntry(currentEntry) dance. Okay. I'll make this a helper. 1025740 21 saam 2014-07-30 10:50:42 -0700 (In reply to comment #20) > (In reply to comment #18) > > (From update of attachment 235724 [details] [details]) > > Actually it might be best to make a helper to do this. There are a couple other places where we do the _hideEntry(previousEntry) / _showEntry(currentEntry) dance. > > Okay. I'll make this a helper. It actually doesn't seem like there is a good way to generalize this to all other place that call both _showEntry/_hideEntry 1025746 22 235750 saam 2014-07-30 10:59:56 -0700 Created attachment 235750 another patch proposition This could also be a way to implement that patch, that passes in a flag indicating whether BackForwardEntry should call shown(). Which do you all like better? 1025887 23 235750 timothy 2014-07-30 20:11:38 -0700 Comment on attachment 235750 another patch proposition I like this. 1025911 24 235750 joepeck 2014-07-30 21:39:37 -0700 Comment on attachment 235750 another patch proposition View in context: https://bugs.webkit.org/attachment.cgi?id=235750&action=review > Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js:57 > this._restoreFromCookie(); Did we also determine that _restoreFromCookie might call shown()? That was being tracked separately though. 1026049 25 saam 2014-07-31 13:09:54 -0700 (In reply to comment #24) > (From update of attachment 235750 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235750&action=review > > > Source/WebInspectorUI/UserInterface/Models/BackForwardEntry.js:57 > > this._restoreFromCookie(); > > Did we also determine that _restoreFromCookie might call shown()? That was being tracked separately though. I haven't looked into it, specifically. Anecdotally, this hasn't happened in my testing of the above patches. That's not to say it might not. 1026768 26 236000 saam 2014-08-04 19:04:08 -0700 Created attachment 236000 patch I've tested this patch, and it solves the problem of shown not being repeatedly called when stepping over instructions in the debugger, but it doesn't solve all the problems of shown being called multiple times. I think for further fixing the redundant calls to shown, I should do more investigation on: https://bugs.webkit.org/show_bug.cgi?id=135000. 1026896 27 236000 commit-queue 2014-08-05 11:30:04 -0700 Comment on attachment 236000 patch Clearing flags on attachment: 236000 Committed r172038: <http://trac.webkit.org/changeset/172038> 1026897 28 commit-queue 2014-08-05 11:30:09 -0700 All reviewed patches have been landed. Closing bug. 235724 2014-07-29 19:28:15 -0700 2014-07-30 10:59:56 -0700 Idea for patch patch.diff text/plain 1336 saam SW5kZXg6IFNvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRW aWV3Q29udGFpbmVyLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2Vy SW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVyLmpzCShyZXZpc2lvbiAxNzE2OTMp CisrKyBTb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9Db250ZW50Vmll d0NvbnRhaW5lci5qcwkod29ya2luZyBjb3B5KQpAQCAtMTkwLDE2ICsxOTAsMjEgQEAgV2ViSW5z cGVjdG9yLkNvbnRlbnRWaWV3Q29udGFpbmVyLnByb3RvdAogICAgICAgICBpZiAodGhpcy5fY3Vy cmVudEluZGV4ID09PSBpbmRleCkKICAgICAgICAgICAgIHJldHVybjsKIAotICAgICAgICAvLyBI aWRlIHRoZSBjdXJyZW50bHkgdmlzaWJsZSBjb250ZW50IHZpZXcuCiAgICAgICAgIHZhciBwcmV2 aW91c0VudHJ5ID0gdGhpcy5jdXJyZW50QmFja0ZvcndhcmRFbnRyeTsKLSAgICAgICAgaWYgKHBy ZXZpb3VzRW50cnkpCi0gICAgICAgICAgICB0aGlzLl9oaWRlRW50cnkocHJldmlvdXNFbnRyeSk7 Ci0KICAgICAgICAgdGhpcy5fY3VycmVudEluZGV4ID0gaW5kZXg7CiAgICAgICAgIHZhciBjdXJy ZW50RW50cnkgPSB0aGlzLmN1cnJlbnRCYWNrRm9yd2FyZEVudHJ5OwogICAgICAgICBjb25zb2xl LmFzc2VydChjdXJyZW50RW50cnkpOwogCi0gICAgICAgIHRoaXMuX3Nob3dFbnRyeShjdXJyZW50 RW50cnkpOworICAgICAgICB2YXIgaXNOZXdDb250ZW50VmlldyA9ICFwcmV2aW91c0VudHJ5IHx8 ICFjdXJyZW50RW50cnkuY29udGVudFZpZXcudmlzaWJsZTsKKyAgICAgICAgaWYgKGlzTmV3Q29u dGVudFZpZXcpIHsKKyAgICAgICAgICAgIC8vIEhpZGUgdGhlIGN1cnJlbnRseSB2aXNpYmxlIGNv bnRlbnQgdmlldy4KKyAgICAgICAgICAgIGlmIChwcmV2aW91c0VudHJ5KQorICAgICAgICAgICAg ICAgIHRoaXMuX2hpZGVFbnRyeShwcmV2aW91c0VudHJ5KTsKKyAgICAgICAgICAgIHRoaXMuX3No b3dFbnRyeShjdXJyZW50RW50cnkpOworICAgICAgICB9IGVsc2UgeworICAgICAgICAgICAgY3Vy cmVudEVudHJ5Ll9yZXN0b3JlRnJvbUNvb2tpZSgpOworICAgICAgICAgICAgdGhpcy51cGRhdGVM YXlvdXQoKTsKKyAgICAgICAgfQogCiAgICAgICAgIHRoaXMuZGlzcGF0Y2hFdmVudFRvTGlzdGVu ZXJzKFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRhaW5lci5FdmVudC5DdXJyZW50Q29udGVu dFZpZXdEaWRDaGFuZ2UpOwogICAgIH0sCg== 235750 2014-07-30 10:59:56 -0700 2014-08-04 19:04:08 -0700 another patch proposition patch.diff text/plain 3650 saam SW5kZXg6IFNvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRW aWV3Q29udGFpbmVyLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2Vy SW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVyLmpzCShyZXZpc2lvbiAxNzE2OTMp CisrKyBTb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9Db250ZW50Vmll d0NvbnRhaW5lci5qcwkod29ya2luZyBjb3B5KQpAQCAtMTkwLDE2ICsxOTAsMTkgQEAgV2ViSW5z cGVjdG9yLkNvbnRlbnRWaWV3Q29udGFpbmVyLnByb3RvdAogICAgICAgICBpZiAodGhpcy5fY3Vy cmVudEluZGV4ID09PSBpbmRleCkKICAgICAgICAgICAgIHJldHVybjsKIAotICAgICAgICAvLyBI aWRlIHRoZSBjdXJyZW50bHkgdmlzaWJsZSBjb250ZW50IHZpZXcuCiAgICAgICAgIHZhciBwcmV2 aW91c0VudHJ5ID0gdGhpcy5jdXJyZW50QmFja0ZvcndhcmRFbnRyeTsKLSAgICAgICAgaWYgKHBy ZXZpb3VzRW50cnkpCi0gICAgICAgICAgICB0aGlzLl9oaWRlRW50cnkocHJldmlvdXNFbnRyeSk7 Ci0KICAgICAgICAgdGhpcy5fY3VycmVudEluZGV4ID0gaW5kZXg7CiAgICAgICAgIHZhciBjdXJy ZW50RW50cnkgPSB0aGlzLmN1cnJlbnRCYWNrRm9yd2FyZEVudHJ5OwogICAgICAgICBjb25zb2xl LmFzc2VydChjdXJyZW50RW50cnkpOwogCi0gICAgICAgIHRoaXMuX3Nob3dFbnRyeShjdXJyZW50 RW50cnkpOworICAgICAgICB2YXIgaXNOZXdDb250ZW50VmlldyA9ICFwcmV2aW91c0VudHJ5IHx8 ICFjdXJyZW50RW50cnkuY29udGVudFZpZXcudmlzaWJsZTsKKyAgICAgICAgaWYgKGlzTmV3Q29u dGVudFZpZXcpIHsKKyAgICAgICAgICAgIC8vIEhpZGUgdGhlIGN1cnJlbnRseSB2aXNpYmxlIGNv bnRlbnQgdmlldy4KKyAgICAgICAgICAgIGlmIChwcmV2aW91c0VudHJ5KQorICAgICAgICAgICAg ICAgIHRoaXMuX2hpZGVFbnRyeShwcmV2aW91c0VudHJ5KTsKKyAgICAgICAgICAgIHRoaXMuX3No b3dFbnRyeShjdXJyZW50RW50cnksIHRydWUpOworICAgICAgICB9IGVsc2UKKyAgICAgICAgICAg IHRoaXMuX3Nob3dFbnRyeShjdXJyZW50RW50cnksIGZhbHNlKTsKIAogICAgICAgICB0aGlzLmRp c3BhdGNoRXZlbnRUb0xpc3RlbmVycyhXZWJJbnNwZWN0b3IuQ29udGVudFZpZXdDb250YWluZXIu RXZlbnQuQ3VycmVudENvbnRlbnRWaWV3RGlkQ2hhbmdlKTsKICAgICB9LApAQCAtMjQwLDcgKzI0 Myw3IEBAIFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRhaW5lci5wcm90b3QKIAogICAgICAg ICAvLyBSZS1zaG93IHRoZSBjdXJyZW50IGVudHJ5LCBiZWNhdXNlIGl0cyBjb250ZW50IHZpZXcg aW5zdGFuY2Ugd2FzIHJlcGxhY2VkLgogICAgICAgICBpZiAoY3VycmVudGx5U2hvd2luZykgewot ICAgICAgICAgICAgdGhpcy5fc2hvd0VudHJ5KHRoaXMuY3VycmVudEJhY2tGb3J3YXJkRW50cnkp OworICAgICAgICAgICAgdGhpcy5fc2hvd0VudHJ5KHRoaXMuY3VycmVudEJhY2tGb3J3YXJkRW50 cnksIHRydWUpOwogICAgICAgICAgICAgdGhpcy5kaXNwYXRjaEV2ZW50VG9MaXN0ZW5lcnMoV2Vi SW5zcGVjdG9yLkNvbnRlbnRWaWV3Q29udGFpbmVyLkV2ZW50LkN1cnJlbnRDb250ZW50Vmlld0Rp ZENoYW5nZSk7CiAgICAgICAgIH0KICAgICB9LApAQCAtMjk2LDcgKzI5OSw3IEBAIFdlYkluc3Bl Y3Rvci5Db250ZW50Vmlld0NvbnRhaW5lci5wcm90b3QKICAgICAgICAgY29uc29sZS5hc3NlcnQo Y3VycmVudEVudHJ5IHx8ICghY3VycmVudEVudHJ5ICYmIHRoaXMuX2N1cnJlbnRJbmRleCA9PT0g LTEpKTsKIAogICAgICAgICBpZiAoY3VycmVudEVudHJ5ICYmIGN1cnJlbnRFbnRyeS5jb250ZW50 VmlldyAhPT0gb2xkQ3VycmVudENvbnRlbnRWaWV3IHx8IGJhY2tGb3J3YXJkTGlzdERpZENoYW5n ZSkgewotICAgICAgICAgICAgdGhpcy5fc2hvd0VudHJ5KGN1cnJlbnRFbnRyeSk7CisgICAgICAg ICAgICB0aGlzLl9zaG93RW50cnkoY3VycmVudEVudHJ5LCB0cnVlKTsKICAgICAgICAgICAgIHRo aXMuZGlzcGF0Y2hFdmVudFRvTGlzdGVuZXJzKFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRh aW5lci5FdmVudC5DdXJyZW50Q29udGVudFZpZXdEaWRDaGFuZ2UpOwogICAgICAgICB9CiAgICAg fSwKQEAgLTM1Miw3ICszNTUsNyBAQCBXZWJJbnNwZWN0b3IuQ29udGVudFZpZXdDb250YWluZXIu cHJvdG90CiAgICAgICAgIGlmICghY3VycmVudEVudHJ5KQogICAgICAgICAgICAgcmV0dXJuOwog Ci0gICAgICAgIHRoaXMuX3Nob3dFbnRyeShjdXJyZW50RW50cnkpOworICAgICAgICB0aGlzLl9z aG93RW50cnkoY3VycmVudEVudHJ5LCB0cnVlKTsKICAgICB9LAogCiAgICAgaGlkZGVuOiBmdW5j dGlvbigpCkBAIC0zOTMsMTIgKzM5NiwxMiBAQCBXZWJJbnNwZWN0b3IuQ29udGVudFZpZXdDb250 YWluZXIucHJvdG90CiAgICAgICAgIGNvbnRlbnRWaWV3LmNsb3NlZCgpOwogICAgIH0sCiAKLSAg ICBfc2hvd0VudHJ5OiBmdW5jdGlvbihlbnRyeSkKKyAgICBfc2hvd0VudHJ5OiBmdW5jdGlvbihl bnRyeSwgc2hvdWxkQ2FsbFNob3duKQogICAgIHsKICAgICAgICAgY29uc29sZS5hc3NlcnQoZW50 cnkgaW5zdGFuY2VvZiBXZWJJbnNwZWN0b3IuQmFja0ZvcndhcmRFbnRyeSk7CiAKICAgICAgICAg dGhpcy5fYWRkQ29udGVudFZpZXdFbGVtZW50KGVudHJ5LmNvbnRlbnRWaWV3KTsKLSAgICAgICAg ZW50cnkucHJlcGFyZVRvU2hvdygpOworICAgICAgICBlbnRyeS5wcmVwYXJlVG9TaG93KHNob3Vs ZENhbGxTaG93bik7CiAgICAgfSwKIAogICAgIF9oaWRlRW50cnk6IGZ1bmN0aW9uKGVudHJ5KQpJ bmRleDogU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTW9kZWxzL0JhY2tGb3J3 YXJkRW50cnkuanMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRl cmZhY2UvTW9kZWxzL0JhY2tGb3J3YXJkRW50cnkuanMJKHJldmlzaW9uIDE3MTY5MykKKysrIFNv dXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVscy9CYWNrRm9yd2FyZEVudHJ5 LmpzCSh3b3JraW5nIGNvcHkpCkBAIC01MiwxMiArNTIsMTMgQEAgV2ViSW5zcGVjdG9yLkJhY2tG b3J3YXJkRW50cnkucHJvdG90eXBlIAogICAgICAgICByZXR1cm4gT2JqZWN0LnNoYWxsb3dDb3B5 KHRoaXMuX2Nvb2tpZSk7CiAgICAgfSwKIAotICAgIHByZXBhcmVUb1Nob3c6IGZ1bmN0aW9uKCkK KyAgICBwcmVwYXJlVG9TaG93OiBmdW5jdGlvbihzaG91bGRDYWxsU2hvd24pCiAgICAgewogICAg ICAgICB0aGlzLl9yZXN0b3JlRnJvbUNvb2tpZSgpOwogCiAgICAgICAgIHRoaXMuY29udGVudFZp ZXcudmlzaWJsZSA9IHRydWU7Ci0gICAgICAgIHRoaXMuY29udGVudFZpZXcuc2hvd24oKTsKKyAg ICAgICAgaWYgKHNob3VsZENhbGxTaG93bikKKyAgICAgICAgICAgIHRoaXMuY29udGVudFZpZXcu c2hvd24oKTsKICAgICAgICAgdGhpcy5jb250ZW50Vmlldy51cGRhdGVMYXlvdXQoKTsKICAgICB9 LAo= 236000 2014-08-04 19:04:08 -0700 2014-08-05 11:30:04 -0700 patch patch.diff text/plain 5290 saam SW5kZXg6IFNvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwkocmV2aXNpb24gMTcyMDE3KQorKysgU291cmNl L1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI2IEBA CisyMDE0LTA4LTA0ICBTYWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IFdlYiBJbnNwZWN0b3I6IHNob3duKCkgY2FsbGVkIG9uIGEgY29udGVudCB2aWV3IHdoZW4gc3Rl cHBpbmcgb3ZlciBhbiBpbnN0cnVjdGlvbiBpbiB0aGUgZGVidWdnZXIKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzNTMxMQorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENvbnRlbnRWaWV3Q29udGFpbmVyIHNo b3VsZCBub3QgcmVwZWF0ZWRseSBjYWxsIENvbnRlbnRWaWV3LnByb3RvdHlwZS5zaG93biAKKyAg ICAgICAgb24gQ29udGVudFZpZXdzIHRoYXQgYXJlIGFscmVhZHkgdmlzaWJsZS4gQ29udGVudFZp ZXdDb250YWluZXIgbm93IHBhc3NlcworICAgICAgICBhIGZsYWcgdG8gQmFja0ZvcndhcmRFbnRy eS5wcm90b3R5cGUucHJlcGFyZVRvU2hvdyBpbmRpY2F0aW5nIHdoZXRoZXIgaXQgc2hvdWxkCisg ICAgICAgIGNhbGwgdGhlIHNob3duIGZ1bmN0aW9uIG9uIHRoZSBDb250ZW50VmlldyBpdCBpcyBh Ym91dCB0byBkaXNwbGF5LgorICAgICAgICBDb250ZW50Vmlld0NvbnRhaW5lci5wcm90b3R5cGUu c2hvd0JhY2tGb3J3YXJkRW50cnlGb3JJbmRleCBwYXNzZXMgaW4gdGhpcworICAgICAgICBmbGFn IGJhc2VkIG9uIGl0cyBDb250ZW50VmlldyBiZWluZyB2aXNpYmxlLgorCisgICAgICAgICogVXNl ckludGVyZmFjZS9Nb2RlbHMvQmFja0ZvcndhcmRFbnRyeS5qczoKKyAgICAgICAgKFdlYkluc3Bl Y3Rvci5CYWNrRm9yd2FyZEVudHJ5LnByb3RvdHlwZS5wcmVwYXJlVG9TaG93KToKKyAgICAgICAg KiBVc2VySW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVyLmpzOgorICAgICAgICAo V2ViSW5zcGVjdG9yLkNvbnRlbnRWaWV3Q29udGFpbmVyLnByb3RvdHlwZS5zaG93QmFja0Zvcndh cmRFbnRyeUZvckluZGV4KToKKyAgICAgICAgKFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRh aW5lci5wcm90b3R5cGUucmVwbGFjZUNvbnRlbnRWaWV3KToKKyAgICAgICAgKFdlYkluc3BlY3Rv ci5Db250ZW50Vmlld0NvbnRhaW5lci5wcm90b3R5cGUuY2xvc2VBbGxDb250ZW50Vmlld3NPZlBy b3RvdHlwZSk6CisgICAgICAgIChXZWJJbnNwZWN0b3IuQ29udGVudFZpZXdDb250YWluZXIucHJv dG90eXBlLnNob3duKToKKyAgICAgICAgKFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRhaW5l ci5wcm90b3R5cGUuX3Nob3dFbnRyeSk6CisKIDIwMTQtMDgtMDEgIEpvbmF0aGFuIFdlbGxzICA8 am9ub3dlbGxzQGFwcGxlLmNvbT4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBUaW1lbGluZSBo ZWFkZXIgaGVpZ2h0IGRvZXNuJ3QgbWF0Y2ggc3R5bGUgdXBkYXRlcy4KSW5kZXg6IFNvdXJjZS9X ZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVscy9CYWNrRm9yd2FyZEVudHJ5LmpzCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVscy9C YWNrRm9yd2FyZEVudHJ5LmpzCShyZXZpc2lvbiAxNzIwMTYpCisrKyBTb3VyY2UvV2ViSW5zcGVj dG9yVUkvVXNlckludGVyZmFjZS9Nb2RlbHMvQmFja0ZvcndhcmRFbnRyeS5qcwkod29ya2luZyBj b3B5KQpAQCAtNTIsMTIgKzUyLDEzIEBAIFdlYkluc3BlY3Rvci5CYWNrRm9yd2FyZEVudHJ5LnBy b3RvdHlwZSAKICAgICAgICAgcmV0dXJuIE9iamVjdC5zaGFsbG93Q29weSh0aGlzLl9jb29raWUp OwogICAgIH0sCiAKLSAgICBwcmVwYXJlVG9TaG93OiBmdW5jdGlvbigpCisgICAgcHJlcGFyZVRv U2hvdzogZnVuY3Rpb24oc2hvdWxkQ2FsbFNob3duKQogICAgIHsKICAgICAgICAgdGhpcy5fcmVz dG9yZUZyb21Db29raWUoKTsKIAogICAgICAgICB0aGlzLmNvbnRlbnRWaWV3LnZpc2libGUgPSB0 cnVlOwotICAgICAgICB0aGlzLmNvbnRlbnRWaWV3LnNob3duKCk7CisgICAgICAgIGlmIChzaG91 bGRDYWxsU2hvd24pCisgICAgICAgICAgICB0aGlzLmNvbnRlbnRWaWV3LnNob3duKCk7CiAgICAg ICAgIHRoaXMuY29udGVudFZpZXcudXBkYXRlTGF5b3V0KCk7CiAgICAgfSwKIApJbmRleDogU291 cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvQ29udGVudFZpZXdDb250YWlu ZXIuanMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2Uv Vmlld3MvQ29udGVudFZpZXdDb250YWluZXIuanMJKHJldmlzaW9uIDE3MjAxNikKKysrIFNvdXJj ZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0NvbnRlbnRWaWV3Q29udGFpbmVy LmpzCSh3b3JraW5nIGNvcHkpCkBAIC0xOTAsMTYgKzE5MCwxOSBAQCBXZWJJbnNwZWN0b3IuQ29u dGVudFZpZXdDb250YWluZXIucHJvdG90CiAgICAgICAgIGlmICh0aGlzLl9jdXJyZW50SW5kZXgg PT09IGluZGV4KQogICAgICAgICAgICAgcmV0dXJuOwogCi0gICAgICAgIC8vIEhpZGUgdGhlIGN1 cnJlbnRseSB2aXNpYmxlIGNvbnRlbnQgdmlldy4KICAgICAgICAgdmFyIHByZXZpb3VzRW50cnkg PSB0aGlzLmN1cnJlbnRCYWNrRm9yd2FyZEVudHJ5OwotICAgICAgICBpZiAocHJldmlvdXNFbnRy eSkKLSAgICAgICAgICAgIHRoaXMuX2hpZGVFbnRyeShwcmV2aW91c0VudHJ5KTsKLQogICAgICAg ICB0aGlzLl9jdXJyZW50SW5kZXggPSBpbmRleDsKICAgICAgICAgdmFyIGN1cnJlbnRFbnRyeSA9 IHRoaXMuY3VycmVudEJhY2tGb3J3YXJkRW50cnk7CiAgICAgICAgIGNvbnNvbGUuYXNzZXJ0KGN1 cnJlbnRFbnRyeSk7CiAKLSAgICAgICAgdGhpcy5fc2hvd0VudHJ5KGN1cnJlbnRFbnRyeSk7Cisg ICAgICAgIHZhciBpc05ld0NvbnRlbnRWaWV3ID0gIXByZXZpb3VzRW50cnkgfHwgIWN1cnJlbnRF bnRyeS5jb250ZW50Vmlldy52aXNpYmxlOworICAgICAgICBpZiAoaXNOZXdDb250ZW50Vmlldykg eworICAgICAgICAgICAgLy8gSGlkZSB0aGUgY3VycmVudGx5IHZpc2libGUgY29udGVudCB2aWV3 LgorICAgICAgICAgICAgaWYgKHByZXZpb3VzRW50cnkpCisgICAgICAgICAgICAgICAgdGhpcy5f aGlkZUVudHJ5KHByZXZpb3VzRW50cnkpOworICAgICAgICAgICAgdGhpcy5fc2hvd0VudHJ5KGN1 cnJlbnRFbnRyeSwgdHJ1ZSk7CisgICAgICAgIH0gZWxzZQorICAgICAgICAgICAgdGhpcy5fc2hv d0VudHJ5KGN1cnJlbnRFbnRyeSwgZmFsc2UpOwogCiAgICAgICAgIHRoaXMuZGlzcGF0Y2hFdmVu dFRvTGlzdGVuZXJzKFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRhaW5lci5FdmVudC5DdXJy ZW50Q29udGVudFZpZXdEaWRDaGFuZ2UpOwogICAgIH0sCkBAIC0yNDAsNyArMjQzLDcgQEAgV2Vi SW5zcGVjdG9yLkNvbnRlbnRWaWV3Q29udGFpbmVyLnByb3RvdAogCiAgICAgICAgIC8vIFJlLXNo b3cgdGhlIGN1cnJlbnQgZW50cnksIGJlY2F1c2UgaXRzIGNvbnRlbnQgdmlldyBpbnN0YW5jZSB3 YXMgcmVwbGFjZWQuCiAgICAgICAgIGlmIChjdXJyZW50bHlTaG93aW5nKSB7Ci0gICAgICAgICAg ICB0aGlzLl9zaG93RW50cnkodGhpcy5jdXJyZW50QmFja0ZvcndhcmRFbnRyeSk7CisgICAgICAg ICAgICB0aGlzLl9zaG93RW50cnkodGhpcy5jdXJyZW50QmFja0ZvcndhcmRFbnRyeSwgdHJ1ZSk7 CiAgICAgICAgICAgICB0aGlzLmRpc3BhdGNoRXZlbnRUb0xpc3RlbmVycyhXZWJJbnNwZWN0b3Iu Q29udGVudFZpZXdDb250YWluZXIuRXZlbnQuQ3VycmVudENvbnRlbnRWaWV3RGlkQ2hhbmdlKTsK ICAgICAgICAgfQogICAgIH0sCkBAIC0yOTYsNyArMjk5LDcgQEAgV2ViSW5zcGVjdG9yLkNvbnRl bnRWaWV3Q29udGFpbmVyLnByb3RvdAogICAgICAgICBjb25zb2xlLmFzc2VydChjdXJyZW50RW50 cnkgfHwgKCFjdXJyZW50RW50cnkgJiYgdGhpcy5fY3VycmVudEluZGV4ID09PSAtMSkpOwogCiAg ICAgICAgIGlmIChjdXJyZW50RW50cnkgJiYgY3VycmVudEVudHJ5LmNvbnRlbnRWaWV3ICE9PSBv bGRDdXJyZW50Q29udGVudFZpZXcgfHwgYmFja0ZvcndhcmRMaXN0RGlkQ2hhbmdlKSB7Ci0gICAg ICAgICAgICB0aGlzLl9zaG93RW50cnkoY3VycmVudEVudHJ5KTsKKyAgICAgICAgICAgIHRoaXMu X3Nob3dFbnRyeShjdXJyZW50RW50cnksIHRydWUpOwogICAgICAgICAgICAgdGhpcy5kaXNwYXRj aEV2ZW50VG9MaXN0ZW5lcnMoV2ViSW5zcGVjdG9yLkNvbnRlbnRWaWV3Q29udGFpbmVyLkV2ZW50 LkN1cnJlbnRDb250ZW50Vmlld0RpZENoYW5nZSk7CiAgICAgICAgIH0KICAgICB9LApAQCAtMzUy LDcgKzM1NSw3IEBAIFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRhaW5lci5wcm90b3QKICAg ICAgICAgaWYgKCFjdXJyZW50RW50cnkpCiAgICAgICAgICAgICByZXR1cm47CiAKLSAgICAgICAg dGhpcy5fc2hvd0VudHJ5KGN1cnJlbnRFbnRyeSk7CisgICAgICAgIHRoaXMuX3Nob3dFbnRyeShj dXJyZW50RW50cnksIHRydWUpOwogICAgIH0sCiAKICAgICBoaWRkZW46IGZ1bmN0aW9uKCkKQEAg LTM5MywxMiArMzk2LDEyIEBAIFdlYkluc3BlY3Rvci5Db250ZW50Vmlld0NvbnRhaW5lci5wcm90 b3QKICAgICAgICAgY29udGVudFZpZXcuY2xvc2VkKCk7CiAgICAgfSwKIAotICAgIF9zaG93RW50 cnk6IGZ1bmN0aW9uKGVudHJ5KQorICAgIF9zaG93RW50cnk6IGZ1bmN0aW9uKGVudHJ5LCBzaG91 bGRDYWxsU2hvd24pCiAgICAgewogICAgICAgICBjb25zb2xlLmFzc2VydChlbnRyeSBpbnN0YW5j ZW9mIFdlYkluc3BlY3Rvci5CYWNrRm9yd2FyZEVudHJ5KTsKIAogICAgICAgICB0aGlzLl9hZGRD b250ZW50Vmlld0VsZW1lbnQoZW50cnkuY29udGVudFZpZXcpOwotICAgICAgICBlbnRyeS5wcmVw YXJlVG9TaG93KCk7CisgICAgICAgIGVudHJ5LnByZXBhcmVUb1Nob3coc2hvdWxkQ2FsbFNob3du KTsKICAgICB9LAogCiAgICAgX2hpZGVFbnRyeTogZnVuY3Rpb24oZW50cnkpCg==