Changeset 224081 in webkit


Ignore:
Timestamp:
Oct 26, 2017 7:20:05 PM (6 years ago)
Author:
webkit@devinrousso.com
Message:

Web Inspector: Canvas Tab: canvas path components from old page stick around when page is reloaded
​https://bugs.webkit.org/show_bug.cgi?id=178806
<rdar://problem/35176360>

Reviewed by Brian Burg.

Source/WebInspectorUI:

Before this change, the CanvasCollection was regenerated each time the Canvas tab was
attached. This also caused the invisible TreeOutline, used for the path components and to
manage selection, to be reset as well. Whenever the page refreshed, however, the old
canvases, and associated recordings, were not removed. This patch reworks all of this logic
to create the CanvasCollection on construction of the tab and add/remove canvases as needed.

  • UserInterface/Controllers/CanvasManager.js:

(WI.CanvasManager.prototype.canvasAdded):
(WI.CanvasManager.prototype.canvasRemoved):
(WI.CanvasManager.prototype._removeCanvas):
(WI.CanvasManager.prototype._mainResourceDidChange):
Replaced the "Cleared" event by instead firing "CanvasRemoved" for every tracked canvas. This
massively simplifies the logic for when the page navigates, as everything goes through a
single event listener.
Drive-by: changed existing event names to remove unnecessary words.

  • UserInterface/Views/CanvasTabContentView.js:

(WI.CanvasTabContentView):
(WI.CanvasTabContentView.prototype.attached):
(WI.CanvasTabContentView.prototype.detached):
(WI.CanvasTabContentView.prototype._addCanvas):
(WI.CanvasTabContentView.prototype._removeCanvas):
(WI.CanvasTabContentView.prototype._handleCanvasAdded):
(WI.CanvasTabContentView.prototype._handleCanvasRemoved):
(WI.CanvasTabContentView.prototype._canvasAdded): Deleted.
(WI.CanvasTabContentView.prototype._canvasRemoved): Deleted.
(WI.CanvasTabContentView.prototype._mainResourceDidChange): Deleted.
Rework logic for the way that the CanvasCollection is maintained. It is no longer
created/destroyed each time the view is attached/detached, and instead a diff is calculated
for the added/existing/removed canvases and the content views are added/removed as such.

  • UserInterface/Views/CanvasOverviewContentView.js:

(WI.CanvasOverviewContentView.prototype._selectionPathComponentsChanged):
Drive-by: if an imported recording is selected on the Canvas Overview view, we should show it.

LayoutTests:

  • inspector/canvas/context-attributes.html:
  • inspector/canvas/resources/create-context-utilities.js:

(destroyCanvases):
(TestPage.registerInitializer.awaitCanvasAdded):
(TestPage.registerInitializer):

  • inspector/canvas/resources/shaderProgram-utilities.js:

(deleteContext):
(TestPage.registerInitializer.window.addParentCanvasRemovedTestCase):
(TestPage.registerInitializer):

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r224078 r224081  
     12017-10-26  Devin Rousso  <webkit@devinrousso.com>
     2
     3        Web Inspector: Canvas Tab: canvas path components from old page stick around when page is reloaded
     4        https://bugs.webkit.org/show_bug.cgi?id=178806
     5        <rdar://problem/35176360>
     6
     7        Reviewed by Brian Burg.
     8
     9        * inspector/canvas/context-attributes.html:
     10        * inspector/canvas/resources/create-context-utilities.js:
     11        (destroyCanvases):
     12        (TestPage.registerInitializer.awaitCanvasAdded):
     13        (TestPage.registerInitializer):
     14        * inspector/canvas/resources/shaderProgram-utilities.js:
     15        (deleteContext):
     16        (TestPage.registerInitializer.window.addParentCanvasRemovedTestCase):
     17        (TestPage.registerInitializer):
     18
    1192017-10-25  Simon Fraser  <simon.fraser@apple.com>
    220

  • trunk/LayoutTests/inspector/canvas/context-attributes.html

    r220119 r224081  
    2121            description: "Check that created canvases have the correct type and attributes sent to the frontend.",
    2222            test(resolve, reject) {
    23                 WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasAdded)
     23                WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasAdded)
    2424                .then((event) => {
    2525                    let canvas = event.data.canvas;

  • trunk/LayoutTests/inspector/canvas/resources/create-context-utilities.js

    r220119 r224081  
    2424
    2525    // Force GC to make sure the canvas element is destroyed, otherwise the frontend
    26     // does not receive WI.CanvasManager.Event.CanvasWasRemoved events.
     26    // does not receive WI.CanvasManager.Event.CanvasRemoved events.
    2727    setTimeout(() => { GCController.collect(); }, 0);
    2828}
    … …  
    3232
    3333    function awaitCanvasAdded(contextType) {
    34         return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasAdded)
     34        return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasAdded)
    3535        .then((event) => {
    3636            let canvas = event.data.canvas;
    … …  
    4343
    4444    function awaitCanvasRemoved(canvasIdentifier) {
    45         return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasRemoved)
     45        return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasRemoved)
    4646        .then((event) => {
    4747            let canvas = event.data.canvas;

  • trunk/LayoutTests/inspector/canvas/resources/shaderProgram-utilities.js

    r220294 r224081  
    3838    context = null;
    3939    // Force GC to make sure the canvas element is destroyed, otherwise the frontend
    40     // does not receive WI.CanvasManager.Event.CanvasWasRemoved events.
     40    // does not receive WI.CanvasManager.Event.CanvasRemoved events.
    4141    setTimeout(() => { GCController.collect(); }, 0);
    4242}
    … …  
    113113                        resolve();
    114114                    }),
    115                     WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasRemoved)
     115                    WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasRemoved)
    116116                    .then(reject)
    117117                ])

  • trunk/Source/WebInspectorUI/ChangeLog

    r224067 r224081  
     12017-10-26  Devin Rousso  <webkit@devinrousso.com>
     2
     3        Web Inspector: Canvas Tab: canvas path components from old page stick around when page is reloaded
     4        https://bugs.webkit.org/show_bug.cgi?id=178806
     5        <rdar://problem/35176360>
     6
     7        Reviewed by Brian Burg.
     8
     9        Before this change, the CanvasCollection was regenerated each time the Canvas tab was
     10        attached. This also caused the invisible TreeOutline, used for the path components and to
     11        manage selection, to be reset as well. Whenever the page refreshed, however, the old
     12        canvases, and associated recordings, were not removed. This patch reworks all of this logic
     13        to create the CanvasCollection on construction of the tab and add/remove canvases as needed.
     14
     15        * UserInterface/Controllers/CanvasManager.js:
     16        (WI.CanvasManager.prototype.canvasAdded):
     17        (WI.CanvasManager.prototype.canvasRemoved):
     18        (WI.CanvasManager.prototype._removeCanvas):
     19        (WI.CanvasManager.prototype._mainResourceDidChange):
     20        Replaced the "Cleared" event by instead firing "CanvasRemoved" for every tracked canvas. This
     21        massively simplifies the logic for when the page navigates, as everything goes through a
     22        single event listener.
     23        Drive-by: changed existing event names to remove unnecessary words.
     24
     25        * UserInterface/Views/CanvasTabContentView.js:
     26        (WI.CanvasTabContentView):
     27        (WI.CanvasTabContentView.prototype.attached):
     28        (WI.CanvasTabContentView.prototype.detached):
     29        (WI.CanvasTabContentView.prototype._addCanvas):
     30        (WI.CanvasTabContentView.prototype._removeCanvas):
     31        (WI.CanvasTabContentView.prototype._handleCanvasAdded):
     32        (WI.CanvasTabContentView.prototype._handleCanvasRemoved):
     33        (WI.CanvasTabContentView.prototype._canvasAdded): Deleted.
     34        (WI.CanvasTabContentView.prototype._canvasRemoved): Deleted.
     35        (WI.CanvasTabContentView.prototype._mainResourceDidChange): Deleted.
     36        Rework logic for the way that the CanvasCollection is maintained. It is no longer
     37        created/destroyed each time the view is attached/detached, and instead a diff is calculated
     38        for the added/existing/removed canvases and the content views are added/removed as such.
     39
     40        * UserInterface/Views/CanvasOverviewContentView.js:
     41        (WI.CanvasOverviewContentView.prototype._selectionPathComponentsChanged):
     42        Drive-by: if an imported recording is selected on the Canvas Overview view, we should show it.
     43
    1442017-10-26  Devin Rousso  <webkit@devinrousso.com>
    245

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

    r223952 r224081  
    100100        this._canvasIdentifierMap.set(canvas.identifier, canvas);
    101101
    102         this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasWasAdded, {canvas});
     102        this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasAdded, {canvas});
    103103    }
    104104
    … …  
    112112            return;
    113113
     114        this._removeCanvas(canvas);
     115    }
     116
     117    canvasMemoryChanged(canvasIdentifier, memoryCost)
     118    {
     119        // Called from WI.CanvasObserver.
     120
     121        let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
     122        console.assert(canvas);
     123        if (!canvas)
     124            return;
     125
     126        canvas.memoryCost = memoryCost;
     127    }
     128
     129    cssCanvasClientNodesChanged(canvasIdentifier)
     130    {
     131        // Called from WI.CanvasObserver.
     132
     133        let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
     134        console.assert(canvas);
     135        if (!canvas)
     136            return;
     137
     138        canvas.cssCanvasClientNodesChanged();
     139    }
     140
     141    recordingFinished(canvasIdentifier, recordingPayload)
     142    {
     143        // Called from WI.CanvasObserver.
     144
     145        this._recordingCanvas = null;
     146
     147        let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
     148        console.assert(canvas);
     149        if (!canvas)
     150            return;
     151
     152        let recording = recordingPayload ? WI.Recording.fromPayload(recordingPayload) : null;
     153        if (recording) {
     154            recording.source = canvas;
     155            recording.createDisplayName();
     156
     157            canvas.recordingCollection.add(recording);
     158        }
     159
     160        this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStopped, {canvas, recording});
     161    }
     162
     163    programCreated(canvasIdentifier, programIdentifier)
     164    {
     165        // Called from WI.CanvasObserver.
     166
     167        let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
     168        console.assert(canvas);
     169        if (!canvas)
     170            return;
     171
     172        console.assert(!this._shaderProgramIdentifierMap.has(programIdentifier), `ShaderProgram already exists with id ${programIdentifier}.`);
     173
     174        let program = new WI.ShaderProgram(programIdentifier, canvas);
     175        this._shaderProgramIdentifierMap.set(program.identifier, program);
     176
     177        canvas.shaderProgramCollection.add(program);
     178
     179        this.dispatchEventToListeners(WI.CanvasManager.Event.ShaderProgramAdded, {program});
     180    }
     181
     182    programDeleted(programIdentifier)
     183    {
     184        // Called from WI.CanvasObserver.
     185
     186        let program = this._shaderProgramIdentifierMap.take(programIdentifier);
     187        console.assert(program);
     188        if (!program)
     189            return;
     190
     191        program.canvas.shaderProgramCollection.remove(program);
     192
     193        this._dispatchShaderProgramRemoved(program);
     194    }
     195
     196    // Private
     197
     198    _removeCanvas(canvas)
     199    {
    114200        for (let program of canvas.shaderProgramCollection.items) {
    115201            this._shaderProgramIdentifierMap.delete(program.identifier);
    … …  
    117203        }
    118204
    119         this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasWasRemoved, {canvas});
    120     }
    121 
    122     canvasMemoryChanged(canvasIdentifier, memoryCost)
    123     {
    124         // Called from WI.CanvasObserver.
    125 
    126         let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
    127         console.assert(canvas);
    128         if (!canvas)
    129             return;
    130 
    131         canvas.memoryCost = memoryCost;
    132     }
    133 
    134     cssCanvasClientNodesChanged(canvasIdentifier)
    135     {
    136         // Called from WI.CanvasObserver.
    137 
    138         let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
    139         console.assert(canvas);
    140         if (!canvas)
    141             return;
    142 
    143         canvas.cssCanvasClientNodesChanged();
    144     }
    145 
    146     recordingFinished(canvasIdentifier, recordingPayload)
    147     {
    148         // Called from WI.CanvasObserver.
    149 
    150         this._recordingCanvas = null;
    151 
    152         let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
    153         console.assert(canvas);
    154         if (!canvas)
    155             return;
    156 
    157         let recording = recordingPayload ? WI.Recording.fromPayload(recordingPayload) : null;
    158         if (recording) {
    159             recording.source = canvas;
    160             recording.createDisplayName();
    161 
    162             canvas.recordingCollection.add(recording);
    163         }
    164 
    165         this.dispatchEventToListeners(WI.CanvasManager.Event.RecordingStopped, {canvas, recording});
    166     }
    167 
    168     programCreated(canvasIdentifier, programIdentifier)
    169     {
    170         // Called from WI.CanvasObserver.
    171 
    172         let canvas = this._canvasIdentifierMap.get(canvasIdentifier);
    173         console.assert(canvas);
    174         if (!canvas)
    175             return;
    176 
    177         console.assert(!this._shaderProgramIdentifierMap.has(programIdentifier), `ShaderProgram already exists with id ${programIdentifier}.`);
    178 
    179         let program = new WI.ShaderProgram(programIdentifier, canvas);
    180         this._shaderProgramIdentifierMap.set(program.identifier, program);
    181 
    182         canvas.shaderProgramCollection.add(program);
    183 
    184         this.dispatchEventToListeners(WI.CanvasManager.Event.ShaderProgramAdded, {program});
    185     }
    186 
    187     programDeleted(programIdentifier)
    188     {
    189         // Called from WI.CanvasObserver.
    190 
    191         let program = this._shaderProgramIdentifierMap.take(programIdentifier);
    192         console.assert(program);
    193         if (!program)
    194             return;
    195 
    196         program.canvas.shaderProgramCollection.remove(program);
    197 
    198         this._dispatchShaderProgramRemoved(program);
    199     }
    200 
    201     // Private
     205        this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasRemoved, {canvas});
     206    }
    202207
    203208    _mainResourceDidChange(event)
    … …  
    209214        WI.Canvas.resetUniqueDisplayNameNumbers();
    210215
     216        for (let canvas of this._canvasIdentifierMap.values())
     217            this._removeCanvas(canvas);
     218
    211219        this._shaderProgramIdentifierMap.clear();
    212 
    213         if (this._canvasIdentifierMap.size) {
    214             this._canvasIdentifierMap.clear();
    215             this.dispatchEventToListeners(WI.CanvasManager.Event.Cleared);
    216         }
     220        this._canvasIdentifierMap.clear();
    217221    }
    218222
    … …  
    224228
    225229WI.CanvasManager.Event = {
    226     Cleared: "canvas-manager-cleared",
    227     CanvasWasAdded: "canvas-manager-canvas-was-added",
    228     CanvasWasRemoved: "canvas-manager-canvas-was-removed",
     230    CanvasAdded: "canvas-manager-canvas-was-added",
     231    CanvasRemoved: "canvas-manager-canvas-was-removed",
    229232    RecordingStarted: "canvas-manager-recording-started",
    230233    RecordingStopped: "canvas-manager-recording-stopped",

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

    r224067 r224081  
    197197        if (pathComponent.representedObject instanceof WI.Canvas)
    198198            this.setSelectedItem(pathComponent.representedObject);
     199        else if (pathComponent.representedObject instanceof WI.Recording)
     200            WI.showRepresentedObject(pathComponent.representedObject);
    199201    }
    200202

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

    r223952 r224081  
    3838        super("canvas", ["canvas"], tabBarItem, navigationSidebarPanelConstructor, detailsSidebarPanelConstructors, disableBackForward);
    3939
    40         this._canvasCollection = null;
     40        this._canvasCollection = new WI.CanvasCollection;
    4141
    4242        this._canvasTreeOutline = new WI.TreeOutline;
    … …  
    144144        super.attached();
    145145
    146         WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasWasAdded, this._canvasAdded, this);
    147         WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasWasRemoved, this._canvasRemoved, this);
     146        WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasAdded, this._handleCanvasAdded, this);
     147        WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasRemoved, this._handleCanvasRemoved, this);
    148148        WI.canvasManager.addEventListener(WI.CanvasManager.Event.RecordingStopped, this._recordingStopped, this);
    149         WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
    150149        WI.RecordingContentView.addEventListener(WI.RecordingContentView.Event.RecordingActionIndexChanged, this._recordingActionIndexChanged, this);
    151150
    152         this._canvasCollection = new WI.CanvasCollection(WI.canvasManager.canvases);
     151        let canvases = new Set(Array.from(this._canvasCollection.items).concat(WI.canvasManager.canvases));
    153152
    154153        for (let canvas of this._canvasCollection.items) {
    155             this._canvasTreeOutline.appendChild(new WI.CanvasTreeElement(canvas));
    156 
    157             for (let recording of canvas.recordingCollection.items)
    158                 this._recordingAdded(recording, {suppressShowRecording: true});
     154            if (!canvases.has(canvas))
     155                this._removeCanvas(canvas);
     156        }
     157
     158        for (let canvas of canvases) {
     159            if (!this._canvasCollection.items.has(canvas))
     160                this._addCanvas(canvas);
    159161        }
    160162    }
    … …  
    163165    {
    164166        WI.canvasManager.removeEventListener(null, null, this);
    165         WI.Frame.removeEventListener(null, null, this);
    166167        WI.RecordingContentView.removeEventListener(null, null, this);
    167168
    168         this._canvasCollection = null;
    169 
    170         this._canvasTreeOutline.removeChildren();
    171 
    172169        super.detached();
    173170    }
    … …  
    175172    // Private
    176173
    177     _canvasAdded(event)
    178     {
    179         let canvas = event.data.canvas;
     174    _addCanvas(canvas)
     175    {
    180176        this._canvasTreeOutline.appendChild(new WI.CanvasTreeElement(canvas));
    181177        this._canvasCollection.add(canvas);
    182     }
    183 
    184     _canvasRemoved(event)
    185     {
    186         let canvas = event.data.canvas;
     178
     179        for (let recording of canvas.recordingCollection.items)
     180            this._recordingAdded(recording, {suppressShowRecording: true});
     181    }
     182
     183    _removeCanvas(canvas)
     184    {
     185        // Move all existing recordings for the removed canvas to be imported recordings, as the
     186        // recording's source is no longer valid.
     187        for (let recording of canvas.recordingCollection.items) {
     188            recording.source = null;
     189            recording.createDisplayName();
     190
     191            const subtitle = null;
     192            this._canvasTreeOutline.appendChild(new WI.GeneralTreeElement(["recording"], recording.displayName, subtitle, recording));
     193        }
     194
    187195        let treeElement = this._canvasTreeOutline.findTreeElement(canvas);
    188196        console.assert(treeElement, "Missing tree element for canvas.", canvas);
    189197        this._canvasTreeOutline.removeChild(treeElement);
    190198        this._canvasCollection.remove(canvas);
     199
     200        let currentContentView = this.contentBrowser.currentContentView;
     201        if (currentContentView instanceof WI.RecordingContentView && canvas.recordingCollection.items.has(currentContentView.representedObject))
     202            this.contentBrowser.updateHierarchicalPathForCurrentContentView();
     203    }
     204
     205    _handleCanvasAdded(event)
     206    {
     207        this._addCanvas(event.data.canvas);
     208    }
     209
     210    _handleCanvasRemoved(event)
     211    {
     212        this._removeCanvas(event.data.canvas);
    191213    }
    192214
    … …  
    214236
    215237        console.assert(false, "Unexpected representedObject.", representedObject);
    216     }
    217 
    218     _mainResourceDidChange(event)
    219     {
    220         if (!event.target.isMainFrame())
    221             return;
    222 
    223         this._canvasCollection.clear();
    224238    }
    225239
Note: See TracChangeset for help on using the changeset viewer.