Changeset 287590 in webkit


Ignore:
Timestamp:
Jan 4, 2022 2:51:18 PM (7 months ago)
Author:
Devin Rousso
Message:

Web Inspector: Sources: expanding a grouping of blackboxed call frames should be persistent
​https://bugs.webkit.org/show_bug.cgi?id=234614
<rdar://problem/86989232>

Reviewed by Patrick Angle.

Source/WebInspectorUI:

If the developer has explicitly decided to show blackboxed call frames, we should respect
that decision. Requiring them to re-expand after _every_ debugger action is very hostile.
So long as the blackboxed call frames remain the same we should persist the expansion. Only
if the location of those blackboxed frames changes should we re-collapse.

  • UserInterface/Controllers/DebuggerManager.js:

(WI.DebuggerManager):
(WI.DebuggerManager.prototype.rememberBlackboxedCallFrameGroupToAutoExpand): Added.
(WI.DebuggerManager.prototype.shouldAutoExpandBlackboxedCallFrameGroup): Added.
(WI.DebuggerManager.prototype._didResumeInternal):

  • UserInterface/Views/BlackboxedGroupTreeElement.js:

(WI.BlackboxedGroupTreeElement.prototype.expand):
Keep a list of blackboxed call frame groupings that have been expanded by the developer.
Use this list to decide whether "new" blackboxed call frame grouping should be auto-expanded
by default (i.e. if every WI.CallFrame is identical with something in the list). Clear the
list when resuming, as we're only trying to keep expanded blackboxed call frame groupings
within the same call stack (i.e. while stepping).

  • UserInterface/Base/Utilities.js:

(Array.prototype.groupBy): Deleted.

  • UserInterface/Views/ThreadTreeElement.js:

(WI.ThreadTreeElement.prototype.refresh):
Remove the custom Array.prototype.groupBy since it's no longer needed (and there's now a
builtin function with the same name). Instead, just keep an index of the first blackboxed
call frame and iterate until either the end of the call stack or a non-blackboxed call frame
is found, sliceing from the saved index to the current item to make a blackboxed call
frame group. If that blackboxed call frame group should be auto-expanded (see above), create
a WI.CallFrameTreeElement for each blackboxed call frame instead of one WI.BlackboxedGroupTreeElement.

  • UserInterface/Models/CallFrame.js:

(WI.CallFrame.prototype.isEqual): Added.
Add a convenience function to compare two WI.CallFrame. Right now it just looks at the
WI.SourceCodeLocation, since for most purposes that's really how one would identify a
WI.CallFrame (i.e. it's probably more important to know "is this the same spot in code"
as opposed to "do we have the same this object").

LayoutTests:

  • inspector/unit-tests/array-utilities.html:
  • inspector/unit-tests/array-utilities-expected.txt:

Remove the custom Array.prototype.groupBy since it's no longer needed (and there's now a
builtin function with the same name).

Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r287583 r287590  
     12022-01-04  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: Sources: expanding a grouping of blackboxed call frames should be persistent
     4        https://bugs.webkit.org/show_bug.cgi?id=234614
     5        <rdar://problem/86989232>
     6
     7        Reviewed by Patrick Angle.
     8
     9        * inspector/unit-tests/array-utilities.html:
     10        * inspector/unit-tests/array-utilities-expected.txt:
     11        Remove the custom `Array.prototype.groupBy` since it's no longer needed (and there's now a
     12        builtin function with the same name).
     13
    1142022-01-04  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
    215

  • trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt

    r272371 r287590  
    106106[1,2,3,4] => [[1,2],[2,3],[3,4]]
    107107
    108 -- Running test case: Array.prototype.groupBy
    109 [0,1,0,1,1,0,1,1,1,0] => [0,[1],0,[1,1],0,[1,1,1],0]
    110 [0,1,0,1,1,0,1,1,1,0] => [0,1,0,[1,1],0,[1,1,1],0]
    111 [0,1,0,1,1,0,1,1,1,0] => [0,1,0,1,1,0,[1,1,1],0]
    112 [0,1,0,1,1,0,1,1,1,0] => [0,1,0,1,1,0,1,1,1,0]
    113 [0,1,0,1,1,0,1,1,1] => [0,[1],0,[1,1],0,[1,1,1]]
    114 [0,1,0,1,1,0,1,1,1] => [0,1,0,[1,1],0,[1,1,1]]
    115 [0,1,0,1,1,0,1,1,1] => [0,1,0,1,1,0,[1,1,1]]
    116 [0,1,0,1,1,0,1,1,1] => [0,1,0,1,1,0,1,1,1]
    117 
    118108-- Running test case: Array.prototype.remove
    119109PASS: remove should return true when removing a value that exists.

  • trunk/LayoutTests/inspector/unit-tests/array-utilities.html

    r272371 r287590  
    221221
    222222    suite.addTestCase({
    223         name: "Array.prototype.groupBy",
    224         test() {
    225             function log(input, output) {
    226                 InspectorTest.log(JSON.stringify(input) + " => " + JSON.stringify(output));
    227             }
    228 
    229             let arr = [0, 1, 0, 1, 1, 0, 1, 1, 1, 0];
    230             let isOne = (x) => { return x === 1 };
    231 
    232             log(arr, arr.groupBy(isOne, 1));
    233             log(arr, arr.groupBy(isOne, 2));
    234             log(arr, arr.groupBy(isOne, 3));
    235             log(arr, arr.groupBy(isOne, 4));
    236 
    237             let arr2 = [0, 1, 0, 1, 1, 0, 1, 1, 1];
    238 
    239             log(arr2, arr2.groupBy(isOne, 1));
    240             log(arr2, arr2.groupBy(isOne, 2));
    241             log(arr2, arr2.groupBy(isOne, 3));
    242             log(arr2, arr2.groupBy(isOne, 4));
    243 
    244             return true;
    245         }
    246     });
    247 
    248     suite.addTestCase({
    249223        name: "Array.prototype.remove",
    250224        test() {

  • trunk/Source/WebInspectorUI/ChangeLog

    r287586 r287590  
     12022-01-04  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: Sources: expanding a grouping of blackboxed call frames should be persistent
     4        https://bugs.webkit.org/show_bug.cgi?id=234614
     5        <rdar://problem/86989232>
     6
     7        Reviewed by Patrick Angle.
     8
     9        If the developer has explicitly decided to show blackboxed call frames, we should respect
     10        that decision. Requiring them to re-expand after _every_ debugger action is very hostile.
     11        So long as the blackboxed call frames remain the same we should persist the expansion. Only
     12        if the location of those blackboxed frames changes should we re-collapse.
     13
     14        * UserInterface/Controllers/DebuggerManager.js:
     15        (WI.DebuggerManager):
     16        (WI.DebuggerManager.prototype.rememberBlackboxedCallFrameGroupToAutoExpand): Added.
     17        (WI.DebuggerManager.prototype.shouldAutoExpandBlackboxedCallFrameGroup): Added.
     18        (WI.DebuggerManager.prototype._didResumeInternal):
     19        * UserInterface/Views/BlackboxedGroupTreeElement.js:
     20        (WI.BlackboxedGroupTreeElement.prototype.expand):
     21        Keep a list of blackboxed call frame groupings that have been expanded by the developer.
     22        Use this list to decide whether "new" blackboxed call frame grouping should be auto-expanded
     23        by default (i.e. if every `WI.CallFrame` is identical with something in the list). Clear the
     24        list when resuming, as we're only trying to keep expanded blackboxed call frame groupings
     25        within the same call stack (i.e. while stepping).
     26
     27        * UserInterface/Base/Utilities.js:
     28        (Array.prototype.groupBy): Deleted.
     29        * UserInterface/Views/ThreadTreeElement.js:
     30        (WI.ThreadTreeElement.prototype.refresh):
     31        Remove the custom `Array.prototype.groupBy` since it's no longer needed (and there's now a
     32        builtin function with the same name). Instead, just keep an index of the first blackboxed
     33        call frame and iterate until either the end of the call stack or a non-blackboxed call frame
     34        is found, `slice`ing from the saved index to the current item to make a blackboxed call
     35        frame group. If that blackboxed call frame group should be auto-expanded (see above), create
     36        a `WI.CallFrameTreeElement` for each blackboxed call frame instead of one `WI.BlackboxedGroupTreeElement`.
     37
     38        * UserInterface/Models/CallFrame.js:
     39        (WI.CallFrame.prototype.isEqual): Added.
     40        Add a convenience function to compare two `WI.CallFrame`. Right now it just looks at the
     41        `WI.SourceCodeLocation`, since for most purposes that's really how one would identify a
     42        `WI.CallFrame` (i.e. it's probably more important to know "is this the same spot in code"
     43        as opposed to "do we have the same `this` object").
     44
    1452022-01-04  Devin Rousso  <drousso@apple.com>
    246

  • trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js

    r286611 r287590  
    16251625});
    16261626
    1627 Object.defineProperty(Array.prototype, "groupBy",
    1628 {
    1629     value(groupFunction, minGroupSize = 1)
    1630     {
    1631         let result = [];
    1632         let startIndex = null;
    1633 
    1634         let flush = (endIndex) => {
    1635             if (startIndex === null)
    1636                 return;
    1637             let group = this.slice(startIndex, endIndex + 1);
    1638             let adjacentCount = (endIndex + 1) - startIndex;
    1639             if (adjacentCount >= minGroupSize)
    1640                 result.push(group);
    1641             else
    1642                 result.pushAll(group);
    1643         }
    1644 
    1645         this.forEach((item, i) => {
    1646             if (groupFunction(item)) {
    1647                 startIndex ??= i;
    1648                 if (i === this.length - 1)
    1649                     flush(this.length - 1);
    1650             } else {
    1651                 flush(i - 1);
    1652                 result.push(item);
    1653                 startIndex = null;
    1654             }
    1655         });
    1656 
    1657         return result;
    1658     }
    1659 });
    1660 
    16611627Object.defineProperty(Promise, "chain",
    16621628{

  • trunk/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js

    r269359 r287590  
    8181        this._blackboxedPatternsSetting = new WI.Setting("debugger-blackboxed-patterns", []);
    8282        this._blackboxedPatternDataMap = new Map;
     83        this._blackboxedCallFrameGroupsToAutoExpand = [];
    8384
    8485        this._activeCallFrame = null;
    … …  
    531532    }
    532533
     534    rememberBlackboxedCallFrameGroupToAutoExpand(blackboxedCallFrameGroup)
     535    {
     536        console.assert(!this.shouldAutoExpandBlackboxedCallFrameGroup(blackboxedCallFrameGroup), blackboxedCallFrameGroup);
     537
     538        this._blackboxedCallFrameGroupsToAutoExpand.push(blackboxedCallFrameGroup);
     539    }
     540
     541    shouldAutoExpandBlackboxedCallFrameGroup(blackboxedCallFrameGroup)
     542    {
     543        console.assert(WI.settings.experimentalCollapseBlackboxedCallFrames.value);
     544        console.assert(Array.isArray(blackboxedCallFrameGroup) && blackboxedCallFrameGroup.length && blackboxedCallFrameGroup.every((callFrame) => callFrame instanceof WI.CallFrame && callFrame.blackboxed), blackboxedCallFrameGroup);
     545
     546        return this._blackboxedCallFrameGroupsToAutoExpand.some((blackboxedCallFrameGroupToAutoExpand) => {
     547            if (blackboxedCallFrameGroupToAutoExpand.length !== blackboxedCallFrameGroup.length)
     548                return false;
     549
     550            return blackboxedCallFrameGroupToAutoExpand.every((item, i) => item.isEqual(blackboxedCallFrameGroup[i]));
     551        });
     552    }
     553
    533554    get asyncStackTraceDepth()
    534555    {
    … …  
    14191440        }
    14201441
     1442        this._blackboxedCallFrameGroupsToAutoExpand = [];
     1443
    14211444        this.dataForTarget(target).updateForResume();
    14221445

  • trunk/Source/WebInspectorUI/UserInterface/Models/CallFrame.js

    r272371 r287590  
    6565    get isConsoleEvaluation() { return this._isConsoleEvaluation; }
    6666
     67    isEqual(other)
     68    {
     69        if (!other)
     70            return false;
     71
     72        if (this._sourceCodeLocation && other._sourceCodeLocation)
     73            return this._sourceCodeLocation.isEqual(other._sourceCodeLocation);
     74
     75        return false;
     76    }
     77
    6778    saveIdentityToCookie()
    6879    {

  • trunk/Source/WebInspectorUI/UserInterface/Views/BlackboxedGroupTreeElement.js

    r272371 r287590  
    2828    constructor(callFrames)
    2929    {
    30         console.assert(Array.isArray(callFrames) && callFrames.length && callFrames.every((callFrame) => callFrame instanceof WI.CallFrame));
     30        console.assert(!WI.debuggerManager.shouldAutoExpandBlackboxedCallFrameGroup(callFrames), callFrames);
    3131
    3232        const classNames = ["blackboxed-group"];
    … …  
    4949    expand()
    5050    {
     51        WI.debuggerManager.rememberBlackboxedCallFrameGroupToAutoExpand(this._callFrames);
     52
    5153        let index = this.parent.children.indexOf(this);
    5254        for (let i = this._callFrames.length - 1; i >= 0; --i)

  • trunk/Source/WebInspectorUI/UserInterface/Views/ThreadTreeElement.js

    r272371 r287590  
    5858
    5959        let renderCallFrames = (callFrames, shouldSelectActiveFrame) => {
    60             if (WI.settings.experimentalCollapseBlackboxedCallFrames.value)
    61                 callFrames = callFrames.groupBy((callFrame) => callFrame.blackboxed);
     60            let blackboxedCallFrameGroupStartIndex = undefined;
    6261
    63             for (let callFrameOrBlackboxedGroup of callFrames) {
    64                 if (Array.isArray(callFrameOrBlackboxedGroup)) {
    65                     this.appendChild(new WI.BlackboxedGroupTreeElement(callFrameOrBlackboxedGroup));
     62            // Add one extra iteration to handle call stacks that start blackboxed.
     63            for (let i = 0; i < callFrames.length + 1; ++i) {
     64                let callFrame = callFrames[i];
     65
     66                if (callFrame?.blackboxed && WI.settings.experimentalCollapseBlackboxedCallFrames.value) {
     67                    blackboxedCallFrameGroupStartIndex ??= i;
    6668                    continue;
    6769                }
    68                 let callFrameTreeElement = new WI.CallFrameTreeElement(callFrameOrBlackboxedGroup);
    69                 if (shouldSelectActiveFrame && callFrameOrBlackboxedGroup === activeCallFrame)
     70
     71                if (blackboxedCallFrameGroupStartIndex !== undefined) {
     72                    let blackboxedCallFrameGroup = callFrames.slice(blackboxedCallFrameGroupStartIndex, i);
     73                    blackboxedCallFrameGroupStartIndex = undefined;
     74
     75                    if (!WI.debuggerManager.shouldAutoExpandBlackboxedCallFrameGroup(blackboxedCallFrameGroup)) {
     76                        this.appendChild(new WI.BlackboxedGroupTreeElement(blackboxedCallFrameGroup));
     77                        continue;
     78                    }
     79
     80                    for (let blackboxedCallFrame of blackboxedCallFrameGroup)
     81                        this.appendChild(new WI.CallFrameTreeElement(blackboxedCallFrame));
     82                }
     83
     84                if (!callFrame)
     85                    continue;
     86
     87                let callFrameTreeElement = new WI.CallFrameTreeElement(callFrame);
     88                if (shouldSelectActiveFrame && callFrame === activeCallFrame)
    7089                    activeCallFrameTreeElement = callFrameTreeElement;
    7190                this.appendChild(callFrameTreeElement);
Note: See TracChangeset for help on using the changeset viewer.