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

Description Nikita Vasilyev 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.
Comment 1 Radar WebKit Bug Importer 2016-03-17 23:29:13 PDT
<rdar://problem/25234875>
Comment 2 Nikita Vasilyev 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.
Comment 3 Nikita Vasilyev 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.
Comment 4 Timothy Hatcher 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?
Comment 5 Nikita Vasilyev 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?
Comment 6 Nikita Vasilyev 2016-03-22 23:49:52 PDT
Created attachment 274730 [details]
Patch
Comment 7 Nikita Vasilyev 2016-03-23 00:02:02 PDT
Created attachment 274732 [details]
[Animated GIF] With the patch applied
Comment 8 Timothy Hatcher 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.
Comment 9 Nikita Vasilyev 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.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2016-03-24 00:13:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Commit Bot 2016-03-25 13:42:51 PDT
Re-opened since this is blocked by bug 155902
Comment 13 Joseph Pecoraro 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
Comment 14 Nikita Vasilyev 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.
Comment 15 Nikita Vasilyev 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.
Comment 16 Nikita Vasilyev 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`!
Comment 17 Nikita Vasilyev 2016-03-27 22:32:17 PDT
Created attachment 275013 [details]
Patch
Comment 18 Nikita Vasilyev 2016-03-27 22:33:17 PDT
Created attachment 275014 [details]
[Animated GIF] Before the patch
Comment 19 Nikita Vasilyev 2016-03-27 22:33:48 PDT
Created attachment 275015 [details]
[Animated GIF] With the patch applied
Comment 20 WebKit Commit Bot 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>
Comment 21 WebKit Commit Bot 2016-03-28 10:46:07 PDT
All reviewed patches have been landed.  Closing bug.