RESOLVED FIXED 146256
Web Inspector: Nested ContentBrowsers / ContentViewContainers cause too many ContentView updates
https://bugs.webkit.org/show_bug.cgi?id=146256
Summary Web Inspector: Nested ContentBrowsers / ContentViewContainers cause too many ...
Joseph Pecoraro
Reported 2015-06-23 15:45:17 PDT
* SUMMARY Nested ContentBrowsers / ContentViewContainers cause too many ContentView updates. For resources the View hierarchy looks something like this: - TabBrowser - ContentViewContainer - ResourcesTabContentView (1) (ContentBrowserTabContentView with a ContentBrowser) - ContentViewContainer - ResourceClusterContentView (2) (ClusterContentView) - ContentViewContainer - TextResourceContentView (3) - TextEditor In the process of showing each of the 3 ContentViews, actions are taken on its sub-views that repeat over and over depending on the nested level. ContentViewContainer.showContentView triggers BackForwardEntry.prepareToShow, and BackForwardEntry.prepareToShow triggers multiple actions which may cascade: - _restoreFromCookie() may cascade actions to sub ContentViews - shown() may cascade actions to sub ContentViews - updateLayout() may cascade actions to sub ContentViews This situation is not ideal! It would be much better to reduce duplication of work here. * STEPS TO REPRODUCE 1. Load <http://apple.com> 2. Show page source (Web Inspector, Resources Tab, Main Resource selected) 3. Scroll down the text editor 4. Switch to the Elements tab 5. Switch back to the Resources tab => updateLayout was called 6+ times on the TextEditor * NOTES - Here are annotations of the different updateLayout calls I saw in this case: 1. prepareToShow ResourceClusterContentView triggers restoreFromCookie/showContentView for the TextResourceContentView (call shown false) and updateLayout 2. prepareToShow->shown ResourceClusterContentView triggers shown for the TextResourceContentView (call shown true) and updateLayout 3. prepareToShow->shown ResourceClusterContentView triggers another showContentView for TextResourceContentView (call shown false) and updateLayout 4. prepareToShow->updateLayout ResourceClusterContentView triggers another updateLayout for TextResourceContentView 5. prepareToShow->updateLayout Tab ContentView triggers another updateLayout down the cascade to TextResourceContentView 6. Left Sidebar opening (I came from Elements -> Resources) triggers another updateLayout 7. Quick Console claims it resized (probably didn't) triggers another updateLayout
Attachments
Radar WebKit Bug Importer
Comment 1 2015-06-23 15:45:37 PDT
Joseph Pecoraro
Comment 2 2015-06-23 15:47:19 PDT
> 1. prepareToShow ResourceClusterContentView triggers > restoreFromCookie/showContentView for the TextResourceContentView (call > shown false) and updateLayout This one is particularly nasty, because this is the ResourceClusterContentView triggering an updateLayout on its sub-ContentView (TextResourceContentView) without having shown() on that view first. So the elements may not actually be in the DOM, and therefore may get incorrect / unexpected values for its views.
Joseph Pecoraro
Comment 3 2015-06-23 16:05:01 PDT
> 7. Quick Console claims it resized (probably didn't) triggers another updateLayout Eliminating this one as part of: <https://webkit.org/b/146258> Web Inspector: Reduce QuickConsole DidResize events if it did not change
Blaze Burg
Comment 4 2015-11-18 15:13:05 PST
Matt, what's the status of this bug now that we have batched async layout?
Matt Baker
Comment 5 2015-11-18 16:10:37 PST
(In reply to comment #4) > Matt, what's the status of this bug now that we have batched async layout? Batching layouts reduced the updates to around 3 (from around 7). The fix for https://bugs.webkit.org/show_bug.cgi?id=151058 brings the total down to two (the first of which is skipped because the TextEditor hasn't become visible yet). Will close out when the above issue lands.
Note You need to log in before you can comment on or make changes to this bug.