Bug 145080 – Web Inspector: Tab Restoration incorrectly makes ContentViews "shown" in background tabs

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

[Joseph Pecoraro]

* 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])

[Radar WebKit Bug Importer]

<rdar://problem/20983950>

[Joseph Pecoraro]

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])

[Joseph Pecoraro]

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.

[Joseph Pecoraro]

Created attachment 253246 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 253247 [details]
[PATCH] Proposed Fix

[Build Bot]

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

[Build Bot]

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

[Timothy Hatcher]

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.

[Joseph Pecoraro]

Created attachment 253335 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 253335 [details]
[PATCH] For Landing

Clearing flags on attachment: 253335

Committed r184508: <http://trac.webkit.org/changeset/184508>