Bug 153634 – Web Inspector: some background tabs think they are the foreground tab and do unnecessary work

• Component: Web Inspector
• Status: RESOLVED CONFIGURATION CHANGED

[Joseph Pecoraro]

Created attachment 270177 [details]
[IMAGE] Profile of reloading page with inspector

* SUMMARY
Background Tabs waste a lot of time on reload populating editor content.

* STEPS TO REPRODUCE
1. Inspect webkit.org
2. Resources > select a resource
3. Debugger > select a different resource
4. Show Console Tab
5. Reload
  => ~300ms of frontend time (85%) is spent populating source code text editor content for the resources in the background tabs

* NOTES
- It may be fine for the background tabs to fetch the content, but not to process it!
- See attached screenshot for a profile

[Radar WebKit Bug Importer]

<rdar://problem/24404173>

[Joseph Pecoraro]

This seems to be another case where the background view thinks it is visible!

The background tab's SourceCodeTextEditor thinks "this.visible" is true.

[Joseph Pecoraro]

Wow, merely opening the inspector all ContentViews that are initially created are marked as visible:

    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) OverviewTimelineView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) OverviewTimelineView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) OverviewTimelineView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TimelineRecordingContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) OverviewTimelineView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) ConsoleTabContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) LogContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TextResourceContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) ResourceClusterContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TextResourceContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TextResourceContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) FrameDOMTreeContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TextResourceContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) ResourceClusterContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TextResourceContentView
    Resources/Views/ContentView.js:209:22: CONSOLE TRACE visible (true) TextResourceContentView

This was me opening to the Console tab.

This could be contributing to inspector launch time performance issues as well. The background tabs shouldn't need to do anything until they are first interacted with.

[Blaze Burg]

Nice find!

[Matt Baker]

I have a patch in progress for https://webkit.org/b/150741, which pushes visibility state management into the View base class. I'll check that it resolves this issue.

[Timothy Hatcher]

*** Bug 156076 has been marked as a duplicate of this bug. ***

[Blaze Burg]

Seems to be resolved. I attached a profile of Web Inspector^1 while reloading CNN.com, following Joe's steps to reproduce. Codemirror does not appear anywhere in the profile.

[Blaze Burg]

Created attachment 280060 [details]
Clean profile

[Joseph Pecoraro]

This is definitely not fixed. We are just fortunate that most of the work (Formatting, unnecessarily inserting for color widgets in JavaScript, etc) have been eliminated (moved to a Worker, unnecessary work eliminated) as part of the performance work done in the last 6 months.

While the performance impacts of cases we knew about have diminished, its certainly possible that a future issue could be introduced. We also know of correctness issues caused by this (a background tab, thinking it is active, steals a SourceCodeTextEditor from the foreground tab).

I'm going to reopen.

[Blaze Burg]

(In reply to comment #9)
> This is definitely not fixed. We are just fortunate that most of the work
> (Formatting, unnecessarily inserting for color widgets in JavaScript, etc)
> have been eliminated (moved to a Worker, unnecessary work eliminated) as
> part of the performance work done in the last 6 months.
> 
> While the performance impacts of cases we knew about have diminished, its
> certainly possible that a future issue could be introduced. We also know of
> correctness issues caused by this (a background tab, thinking it is active,
> steals a SourceCodeTextEditor from the foreground tab).
> 
> I'm going to reopen.

OK. Can you please retitle appropriately? If you have steps to reproduce the correctness issue, that would be great too.

[Joseph Pecoraro]

In recent months this issues means the Timeline Tab and Network Tab are constantly doing empty rAF loops, and in some cases causing non-stop console assertions. Short term fix on bug 164841, but just another indication that we should fix this.