| Summary: | Web Inspector: Tab Restoration incorrectly makes ContentViews "shown" in background tabs | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||||
| Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | bburg, buildbot, commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, rniwa, timothy, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Attachments: |
|
||||||||||||
Oops, I pasted the duplicate shown for the Resources tab (again, unexpected). But specifically this was the trace for the Debugger tab:
Views/SourceCodeTextEditor.js:100:20: CONSOLE LOG SourceCodeTextEditor.shown http://bogojoker.com/shell/js/easySlider.min.js
Views/SourceCodeTextEditor.js:101:22: CONSOLE TRACE
0: shown(Views/SourceCodeTextEditor.js:101:22)
1: shown(Views/TextResourceContentView.js:104:31)
2: prepareToShow(Models/BackForwardEntry.js:61:35)
3: _showEntry(Views/ContentViewContainer.js:472:28)
4: showBackForwardEntryForIndex(Views/ContentViewContainer.js:212:28)
5: showContentView(Views/ContentViewContainer.js:188:42)
6: _showContentViewForIdentifier(Views/ResourceClusterContentView.js:236:57)
7: restoreFromCookie(Views/ResourceClusterContentView.js:147:61)
8: _restoreFromCookie(Models/BackForwardEntry.js:78:43)
9: prepareToShow(Models/BackForwardEntry.js:57:32)
10: _showEntry(Views/ContentViewContainer.js:472:28)
11: showBackForwardEntryForIndex(Views/ContentViewContainer.js:212:28)
12: showContentView(Views/ContentViewContainer.js:188:42)
13: showContentViewForRepresentedObject(Views/ContentBrowser.js:168:58)
14: showDefaultContentViewForTreeElement(Views/NavigationSidebarPanel.js:196:64)
15: _checkElementsForPendingViewStateCookie(Views/NavigationSidebarPanel.js:701:54)
16: _checkOutlinesForPendingViewStateCookie(Views/NavigationSidebarPanel.js:656:60)
17: finalAttemptToRestoreViewStateFromCookie(Views/NavigationSidebarPanel.js:242:57)
18: finalAttemptToRestoreViewStateFromCookie([native code])
Views/SourceCodeTextEditor.js:100:20: CONSOLE LOG SourceCodeTextEditor.shown http://bogojoker.com/shell/js/easySlider.min.js
Views/SourceCodeTextEditor.js:101:22: CONSOLE TRACE
0: shown(Views/SourceCodeTextEditor.js:101:22)
1: shown(Views/TextResourceContentView.js:104:31)
2: prepareToShow(Models/BackForwardEntry.js:61:35)
3: _showEntry(Views/ContentViewContainer.js:472:28)
4: shown(Views/ContentViewContainer.js:425:24)
5: shown(Views/ClusterContentView.js:75:41)
6: shown(Views/ResourceClusterContentView.js:125:61)
7: prepareToShow(Models/BackForwardEntry.js:61:35)
8: _showEntry(Views/ContentViewContainer.js:472:28)
9: showBackForwardEntryForIndex(Views/ContentViewContainer.js:212:28)
10: showContentView(Views/ContentViewContainer.js:188:42)
11: showContentViewForRepresentedObject(Views/ContentBrowser.js:168:58)
12: showDefaultContentViewForTreeElement(Views/NavigationSidebarPanel.js:196:64)
13: _checkElementsForPendingViewStateCookie(Views/NavigationSidebarPanel.js:701:54)
14: _checkOutlinesForPendingViewStateCookie(Views/NavigationSidebarPanel.js:656:60)
15: finalAttemptToRestoreViewStateFromCookie(Views/NavigationSidebarPanel.js:242:57)
16: finalAttemptToRestoreViewStateFromCookie([native code])
I'll file a separate bug about the duplicate shown. After discussing this with Timothy Hatcher on IRC, he came up with the idea of delaying Tab state restoration until the tab is shown. That is easy to do and seems like a great idea. Instead of potentially initializing the content views of all the tabs on startup, we only initialize the visible tab. Patch soon. Created attachment 253246 [details]
[PATCH] Proposed Fix
Created attachment 253247 [details]
[PATCH] Proposed Fix
Comment on attachment 253247 [details] [PATCH] Proposed Fix Attachment 253247 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6131852188319744 New failing tests: compositing/visible-rect/iframe-no-layers.html Created attachment 253254 [details]
Archive of layout-test-results from ews103 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 253247 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=253247&action=review > Source/WebInspectorUI/UserInterface/Views/TabContentView.js:113 > + this._shouldRestoreStateWhenShown = false; Might be best to do this in restoreStateFromCookie() so it can't be forgotten. Created attachment 253335 [details]
[PATCH] For Landing
Comment on attachment 253335 [details] [PATCH] For Landing Clearing flags on attachment: 253335 Committed r184508: <http://trac.webkit.org/changeset/184508> |
* SUMMARY Tab Restoration incorrectly makes ContentViews "shown" in background tabs. There seem to a be a couple issues with tab restoration that may affect performance. - Tab state restoration triggers "shown" twice on the current content view per-tab' => this seems like wasted work - Tab state restoration leaves ContentViews as "visible" / "shown" in background tabs => if Type Annotators are enabled, this can mean constant ongoing polling, potentially for multiple editors in multiple tabs * STEPS TO REPRODUCE 1. Inspect <http://bogojoker.com/shell/> 2. In Debugger Tab select the "easySlider.min.js" resource => creates a SourceCodeTextEditor 3. Enable the Type Profiler => enables Type Annotations 4. Switch to the Elements Tab => Type Annotations paused 5. Close Inspector => saves state of tabs 6. Open Inspector => Unexpectedly, the background Debugger tab's SourceCodeTextEditor had its "shown" called twice => Unexpectedly, the background Debugger tab's SourceCodeTextEditor is cluttering the protocol with type annotation updates. * NOTES - Duplicate calls to shown: Views/SourceCodeTextEditor.js:100:20: CONSOLE LOG SourceCodeTextEditor.shown http://bogojoker.com/shell/ Views/SourceCodeTextEditor.js:101:22: CONSOLE TRACE 0: shown(Views/SourceCodeTextEditor.js:101:22) 1: shown(Views/TextResourceContentView.js:104:31) 2: prepareToShow(Models/BackForwardEntry.js:61:35) 3: _showEntry(Views/ContentViewContainer.js:472:28) 4: showBackForwardEntryForIndex(Views/ContentViewContainer.js:212:28) 5: showContentView(Views/ContentViewContainer.js:188:42) 6: _showContentViewForIdentifier(Views/ResourceClusterContentView.js:236:57) 7: restoreFromCookie(Views/ResourceClusterContentView.js:147:61) 8: _restoreFromCookie(Models/BackForwardEntry.js:78:43) 9: prepareToShow(Models/BackForwardEntry.js:57:32) 10: _showEntry(Views/ContentViewContainer.js:472:28) 11: showBackForwardEntryForIndex(Views/ContentViewContainer.js:212:28) 12: showContentView(Views/ContentViewContainer.js:188:42) 13: showContentViewForRepresentedObject(Views/ContentBrowser.js:168:58) 14: showDefaultContentViewForTreeElement(Views/NavigationSidebarPanel.js:196:64) 15: delayedWork(Views/ResourceSidebarPanel.js:192:58) 16: delayedWork([native code]) Views/SourceCodeTextEditor.js:100:20: CONSOLE LOG SourceCodeTextEditor.shown http://bogojoker.com/shell/ Views/SourceCodeTextEditor.js:101:22: CONSOLE TRACE 0: shown(Views/SourceCodeTextEditor.js:101:22) 1: shown(Views/TextResourceContentView.js:104:31) 2: prepareToShow(Models/BackForwardEntry.js:61:35) 3: _showEntry(Views/ContentViewContainer.js:472:28) 4: shown(Views/ContentViewContainer.js:425:24) 5: shown(Views/ClusterContentView.js:75:41) 6: shown(Views/ResourceClusterContentView.js:125:61) 7: prepareToShow(Models/BackForwardEntry.js:61:35) 8: _showEntry(Views/ContentViewContainer.js:472:28) 9: showBackForwardEntryForIndex(Views/ContentViewContainer.js:212:28) 10: showContentView(Views/ContentViewContainer.js:188:42) 11: showContentViewForRepresentedObject(Views/ContentBrowser.js:168:58) 12: showDefaultContentViewForTreeElement(Views/NavigationSidebarPanel.js:196:64) 13: delayedWork(Views/ResourceSidebarPanel.js:192:58) 14: delayedWork([native code])