Bug 195104 - Web Inspector: Sources: disabled breakpoints banner should be sticky
Summary: Web Inspector: Sources: disabled breakpoints banner should be sticky
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 195203
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-27 10:10 PST by Devin Rousso
Modified: 2019-03-20 12:37 PDT (History)
6 users (show)

See Also:


Attachments
[Image] Screenshot of Issue (24.76 KB, image/png)
2019-02-27 10:10 PST, Devin Rousso
no flags Details
Patch (14.02 KB, patch)
2019-03-10 21:11 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (15.50 KB, patch)
2019-03-19 17:58 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (15.05 KB, patch)
2019-03-19 18:02 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (14.22 KB, patch)
2019-03-19 18:09 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (14.24 KB, patch)
2019-03-20 11:37 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-02-27 10:10:06 PST
Created attachment 363102 [details]
[Image] Screenshot of Issue

It's not very helpful if you have a lot of breakpoints and want to disable/enable them quickly.
Comment 1 Radar WebKit Bug Importer 2019-02-27 10:51:13 PST
<rdar://problem/48442259>
Comment 2 Devin Rousso 2019-03-10 21:11:49 PDT
Created attachment 364229 [details]
Patch
Comment 3 Joseph Pecoraro 2019-03-14 00:09:11 PDT
Comment on attachment 364229 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364229&action=review

> Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css:125
> +@media (min-height: 600px) {

This is not described in the ChangeLog. What are you changing based on height?

> Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1452
> +    _handleCallStackElementAddedOrRemoved(event)
> +    {
> +        function walk(parent) {
> +            let count = 0;
> +            if (!parent.root)
> +                ++count;
> +
> +            for (let child of parent.children)
> +                count += walk(child);
> +
> +            // Don't count the main thread element when it's hidden.
> +            if (parent instanceof WI.ThreadTreeElement && WI.targets.length === 1)
> +                --count;
> +
> +            return count;
> +        }
> +        this.element.style.setProperty("--call-stack-count", walk(this._callStackTreeOutline));
> +    }

How often does this happen? If huge call stack happens, such as (500 or 1000 elements is not uncommon on rAF loops), will this have been called 500 to walk the ...497,498,499,500 children?

It might be nice to take advantage of the fact that we should know tree outline has a depth of 2, so maybe we could compute faster:

    let count = 0;
    count += this._callStackTreeOutline.children.length;
    for (let threadElement of this._callStackTreeOutline)
        threadElement += this._callStackTreeOutline.children.length;

    // Don't count the main thread element when it is hidden.
    if (WI.targets.length === 1)
        --count;

Or maybe just enumerating the call frames list and not the tree elements.
Comment 4 Devin Rousso 2019-03-14 00:39:40 PDT
Comment on attachment 364229 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364229&action=review

>> Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css:125
>> +@media (min-height: 600px) {
> 
> This is not described in the ChangeLog. What are you changing based on height?

I swear I wrote this down somewhere -.-  Oh well :(

The reason for this is at small heights, if we still were to disallow overflowing we'd end up in situations where each section would only be ~50px tall, which is barely enough to show more than one `WI.TreeElement` with the header of a `WI.DetailsSection`.  I chose 600px because it seemed like a nice number where each section would get an even amount of space, assuming they were all each individually overflowing into their own scrollable areas (e.g. the `.pause-reason-container` wouldn't go over ~80px since it's a single element with a known description length, each of `.call-stack-container` and `.breakpoints-container` are limited to a max of `135px`, leaving a good amount of space for the resources list since the 600px also includes the toolbar/tabbar/prompt/etc).

>> Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:1452
>> +    }
> 
> How often does this happen? If huge call stack happens, such as (500 or 1000 elements is not uncommon on rAF loops), will this have been called 500 to walk the ...497,498,499,500 children?
> 
> It might be nice to take advantage of the fact that we should know tree outline has a depth of 2, so maybe we could compute faster:
> 
>     let count = 0;
>     count += this._callStackTreeOutline.children.length;
>     for (let threadElement of this._callStackTreeOutline)
>         threadElement += this._callStackTreeOutline.children.length;
> 
>     // Don't count the main thread element when it is hidden.
>     if (WI.targets.length === 1)
>         --count;
> 
> Or maybe just enumerating the call frames list and not the tree elements.

I'll rework this (and the one for breakpoints) to add `.children.length` to hopefully make it a bit quicker.  Neither of them should ever have a depth more than 2, so I think that that would be fine.
Comment 5 Devin Rousso 2019-03-19 17:58:06 PDT
Created attachment 365274 [details]
Patch
Comment 6 Devin Rousso 2019-03-19 18:02:47 PDT
Created attachment 365276 [details]
Patch
Comment 7 Devin Rousso 2019-03-19 18:09:10 PDT
Created attachment 365277 [details]
Patch
Comment 8 WebKit Commit Bot 2019-03-20 09:21:52 PDT Comment hidden (obsolete)
Comment 9 Devin Rousso 2019-03-20 11:37:12 PDT
Created attachment 365367 [details]
Patch
Comment 10 WebKit Commit Bot 2019-03-20 12:37:22 PDT
Comment on attachment 365367 [details]
Patch

Clearing flags on attachment: 365367

Committed r243225: <https://trac.webkit.org/changeset/243225>
Comment 11 WebKit Commit Bot 2019-03-20 12:37:24 PDT
All reviewed patches have been landed.  Closing bug.