Bug 155627

Summary: Web Inspector: Large repaints while typing in the console tab
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=155387
Bug Depends on: 145324, 155902    
Bug Blocks: 152220    
Attachments:
Description Flags
[Animated GIF] Bug
none
Patch
timothy: review+, timothy: commit-queue-
[Animated GIF] With the patch applied
none
Patch
none
Patch
none
[Animated GIF] Before the patch
none
[Animated GIF] With the patch applied none

Nikita Vasilyev
Reported 2016-03-17 23:28:52 PDT
Created attachment 274381 [details] [Animated GIF] Bug Large repaints in the split console were fixed in Bug 155387. In the console tab, typing any character still causes a full repaint.
Attachments
[Animated GIF] Bug (287.60 KB, image/gif)
2016-03-17 23:28 PDT, Nikita Vasilyev 		no flags
Details
Patch (4.59 KB, patch)
2016-03-22 23:49 PDT, Nikita Vasilyev 		timothy: review+
timothy: commit-queue-
DetailsFormatted DiffDiff
[Animated GIF] With the patch applied (53.12 KB, image/gif)
2016-03-23 00:02 PDT, Nikita Vasilyev 		no flags
Details
Patch (4.58 KB, patch)
2016-03-23 23:12 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Patch (1.85 KB, patch)
2016-03-27 22:32 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
[Animated GIF] Before the patch (624.32 KB, image/gif)
2016-03-27 22:33 PDT, Nikita Vasilyev 		no flags
Details
[Animated GIF] With the patch applied (108.17 KB, image/gif)
2016-03-27 22:33 PDT, Nikita Vasilyev 		no flags
Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2016-03-17 23:29:13 PDT
<rdar://problem/25234875>
Nikita Vasilyev
Comment 2 2016-03-18 00:21:18 PDT
So far, I was only able to work around this issue by replacing flexbox with `position: absolute`. https://bugs.webkit.org/show_bug.cgi?id=145324#attach_273274 Setting `z-index` or even `transform: translateZ(0)` didn't improve anything.
Nikita Vasilyev
Comment 3 2016-03-21 23:42:48 PDT
Adding the following reduces the paint area to the console prompt: #content { position: absolute; left: 0; top: 0; width: 100%; height: 100%; } However, it breaks sidebars for other tabs.
Timothy Hatcher
Comment 4 2016-03-22 12:33:06 PDT
(In reply to comment #3) > Adding the following reduces the paint area to the console prompt: > > #content { > position: absolute; > left: 0; > top: 0; > width: 100%; > height: 100%; > } > > However, it breaks sidebars for other tabs. Wouldn't a more specific selector work that affects on the console tab?
Nikita Vasilyev
Comment 5 2016-03-22 22:17:56 PDT
(In reply to comment #4) > (In reply to comment #3) > > Adding the following reduces the paint area to the console prompt: > > > > #content { > > position: absolute; > > left: 0; > > top: 0; > > width: 100%; > > height: 100%; > > } > > > > However, it breaks sidebars for other tabs. > > Wouldn't a more specific selector work that affects on the console tab? When changing tabs, we don't change any class names for #content or its parents. For instance, we don't have anything like <body class="tab-console">. Do you think this would be worth doing?
Nikita Vasilyev
Comment 6 2016-03-22 23:49:52 PDT
Created attachment 274730 [details] Patch
Nikita Vasilyev
Comment 7 2016-03-23 00:02:02 PDT
Created attachment 274732 [details] [Animated GIF] With the patch applied
Timothy Hatcher
Comment 8 2016-03-23 07:34:55 PDT
Comment on attachment 274730 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274730&action=review > Source/WebInspectorUI/UserInterface/Base/Main.js:549 > + console.assert(previouslySelectedTabIdentifier); I think this will assert the first time a tab is added and selected, since previouslySelectedTabIdentifier will be "". You should drop this assert and make the following line condition on if (previouslySelectedTabIdentifier). > Source/WebInspectorUI/UserInterface/Views/LogContentView.css:32 > + width: 100%; > + height: 100%; Does right: 0 and bottom: 0 work here instead? We usually don't use container based percents.
Nikita Vasilyev
Comment 9 2016-03-23 23:12:54 PDT
Created attachment 274823 [details] Patch (In reply to comment #8) > Comment on attachment 274730 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=274730&action=review > > > Source/WebInspectorUI/UserInterface/Base/Main.js:549 > > + console.assert(previouslySelectedTabIdentifier); > > I think this will assert the first time a tab is added and selected, since > previouslySelectedTabIdentifier will be "". You should drop this assert and > make the following line condition on if (previouslySelectedTabIdentifier). That's exactly what was happening. Fixed. > > > Source/WebInspectorUI/UserInterface/Views/LogContentView.css:32 > > + width: 100%; > > + height: 100%; > > Does right: 0 and bottom: 0 work here instead? We usually don't use > container based percents. AFAIK, `width/height: 100%` work exactly the same as `bottom/right: 0` in this case. Replaced with bottom/right for consistency.
WebKit Commit Bot
Comment 10 2016-03-24 00:13:07 PDT
Comment on attachment 274823 [details] Patch Clearing flags on attachment: 274823 Committed r198619: <http://trac.webkit.org/changeset/198619>
WebKit Commit Bot
Comment 11 2016-03-24 00:13:11 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 12 2016-03-25 13:42:51 PDT
Re-opened since this is blocked by bug 155902
Joseph Pecoraro
Comment 13 2016-03-25 13:43:39 PDT
This often causes me to get a completely blank/unusable Console tab when switching to the Console Tab for the first time. * STEPS TO REPRODUCE 1. Open Inspector 2. Switch to Timeline Tab 3. Close Inspector 4. Open Inspector 5. Switch to Console Tab => Blank, missing navigation bar, typing does not work
Nikita Vasilyev
Comment 14 2016-03-25 16:28:52 PDT
This seem like a recent WebKit bug. Resizing the window makes the console show up properly as well as toggling `position: absolute`. I'll try to create a reduction.
Nikita Vasilyev
Comment 15 2016-03-27 22:10:52 PDT
I used Timelines to profile repaint times. Before the patch: 5ms repaint. After the patch: 0.1ms repaint (x50 improvement). Configuration: MacBook Pro (Retina, 15-inch, Mid 2014) Resolution: 1440 x 900 (2x Retina) Web Inspector window was detached and was 50% of my screen width.
Nikita Vasilyev
Comment 16 2016-03-27 22:31:18 PDT
Comment on attachment 274730 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274730&action=review >>> Source/WebInspectorUI/UserInterface/Views/LogContentView.css:32 >>> + height: 100%; >> >> Does right: 0 and bottom: 0 work here instead? We usually don't use container based percents. > > AFAIK, `width/height: 100%` work exactly the same as `bottom/right: 0` > in this case. Replaced with bottom/right for consistency. I accidentally discovered that setting `height: 100%` alone reduces flexbox paint areas just as well as `position: absolute`!
Nikita Vasilyev
Comment 17 2016-03-27 22:32:17 PDT
Created attachment 275013 [details] Patch
Nikita Vasilyev
Comment 18 2016-03-27 22:33:17 PDT
Created attachment 275014 [details] [Animated GIF] Before the patch
Nikita Vasilyev
Comment 19 2016-03-27 22:33:48 PDT
Created attachment 275015 [details] [Animated GIF] With the patch applied
WebKit Commit Bot
Comment 20 2016-03-28 10:46:01 PDT
Comment on attachment 275013 [details] Patch Clearing flags on attachment: 275013 Committed r198747: <http://trac.webkit.org/changeset/198747>
WebKit Commit Bot
Comment 21 2016-03-28 10:46:07 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.