Skip to content
Permalink
Browse files
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):


Canonical link: https://commits.webkit.org/195069@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224081 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dcrousso committed Oct 27, 2017
1 parent 8f91f2c commit 02fbb71bcb277311d2cf0a93c92b0f726007665e
Showing 8 changed files with 126 additions and 46 deletions.
@@ -1,3 +1,21 @@
2017-10-26 Devin Rousso <webkit@devinrousso.com>

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.

* 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):

2017-10-25 Simon Fraser <simon.fraser@apple.com>

When navigating back to a page, compositing layers may not use accelerated drawing
@@ -20,7 +20,7 @@
name,
description: "Check that created canvases have the correct type and attributes sent to the frontend.",
test(resolve, reject) {
WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasAdded)
WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasAdded)
.then((event) => {
let canvas = event.data.canvas;
InspectorTest.log("Added canvas.");
@@ -23,15 +23,15 @@ function destroyCanvases() {
contexts = [];

// Force GC to make sure the canvas element is destroyed, otherwise the frontend
// does not receive WI.CanvasManager.Event.CanvasWasRemoved events.
// does not receive WI.CanvasManager.Event.CanvasRemoved events.
setTimeout(() => { GCController.collect(); }, 0);
}

TestPage.registerInitializer(() => {
let suite = null;

function awaitCanvasAdded(contextType) {
return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasAdded)
return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasAdded)
.then((event) => {
let canvas = event.data.canvas;
let contextDisplayName = WI.Canvas.displayNameForContextType(contextType);
@@ -42,7 +42,7 @@ TestPage.registerInitializer(() => {
}

function awaitCanvasRemoved(canvasIdentifier) {
return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasRemoved)
return WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasRemoved)
.then((event) => {
let canvas = event.data.canvas;
InspectorTest.expectEqual(canvas.identifier, canvasIdentifier, "Removed canvas has expected ID.");
@@ -37,7 +37,7 @@ function deleteProgram() {
function deleteContext() {
context = null;
// Force GC to make sure the canvas element is destroyed, otherwise the frontend
// does not receive WI.CanvasManager.Event.CanvasWasRemoved events.
// does not receive WI.CanvasManager.Event.CanvasRemoved events.
setTimeout(() => { GCController.collect(); }, 0);
}

@@ -112,7 +112,7 @@ TestPage.registerInitializer(() => {
InspectorTest.pass("Removed ShaderProgram before Canvas.");
resolve();
}),
WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasWasRemoved)
WI.canvasManager.awaitEvent(WI.CanvasManager.Event.CanvasRemoved)
.then(reject)
])
.catch(() => { InspectorTest.fail("Removed Canvas before ShaderProgram."); });
@@ -1,3 +1,46 @@
2017-10-26 Devin Rousso <webkit@devinrousso.com>

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.

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.

2017-10-26 Devin Rousso <webkit@devinrousso.com>

Web Inspector: Canvas Tab: canvases overview should support navigation via keyboard
@@ -99,7 +99,7 @@ WI.CanvasManager = class CanvasManager extends WI.Object
let canvas = WI.Canvas.fromPayload(canvasPayload);
this._canvasIdentifierMap.set(canvas.identifier, canvas);

this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasWasAdded, {canvas});
this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasAdded, {canvas});
}

canvasRemoved(canvasIdentifier)
@@ -111,12 +111,7 @@ WI.CanvasManager = class CanvasManager extends WI.Object
if (!canvas)
return;

for (let program of canvas.shaderProgramCollection.items) {
this._shaderProgramIdentifierMap.delete(program.identifier);
this._dispatchShaderProgramRemoved(program);
}

this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasWasRemoved, {canvas});
this._removeCanvas(canvas);
}

canvasMemoryChanged(canvasIdentifier, memoryCost)
@@ -200,6 +195,16 @@ WI.CanvasManager = class CanvasManager extends WI.Object

// Private

_removeCanvas(canvas)
{
for (let program of canvas.shaderProgramCollection.items) {
this._shaderProgramIdentifierMap.delete(program.identifier);
this._dispatchShaderProgramRemoved(program);
}

this.dispatchEventToListeners(WI.CanvasManager.Event.CanvasRemoved, {canvas});
}

_mainResourceDidChange(event)
{
console.assert(event.target instanceof WI.Frame);
@@ -208,12 +213,11 @@ WI.CanvasManager = class CanvasManager extends WI.Object

WI.Canvas.resetUniqueDisplayNameNumbers();

this._shaderProgramIdentifierMap.clear();
for (let canvas of this._canvasIdentifierMap.values())
this._removeCanvas(canvas);

if (this._canvasIdentifierMap.size) {
this._canvasIdentifierMap.clear();
this.dispatchEventToListeners(WI.CanvasManager.Event.Cleared);
}
this._shaderProgramIdentifierMap.clear();
this._canvasIdentifierMap.clear();
}

_dispatchShaderProgramRemoved(program)
@@ -223,9 +227,8 @@ WI.CanvasManager = class CanvasManager extends WI.Object
};

WI.CanvasManager.Event = {
Cleared: "canvas-manager-cleared",
CanvasWasAdded: "canvas-manager-canvas-was-added",
CanvasWasRemoved: "canvas-manager-canvas-was-removed",
CanvasAdded: "canvas-manager-canvas-was-added",
CanvasRemoved: "canvas-manager-canvas-was-removed",
RecordingStarted: "canvas-manager-recording-started",
RecordingStopped: "canvas-manager-recording-stopped",
ShaderProgramAdded: "canvas-manager-shader-program-added",
@@ -196,6 +196,8 @@ WI.CanvasOverviewContentView = class CanvasOverviewContentView extends WI.Collec
let pathComponent = event.data.pathComponent;
if (pathComponent.representedObject instanceof WI.Canvas)
this.setSelectedItem(pathComponent.representedObject);
else if (pathComponent.representedObject instanceof WI.Recording)
WI.showRepresentedObject(pathComponent.representedObject);
}

_showGridButtonClicked(event)
@@ -37,7 +37,7 @@ WI.CanvasTabContentView = class CanvasTabContentView extends WI.ContentBrowserTa
const disableBackForward = true;
super("canvas", ["canvas"], tabBarItem, navigationSidebarPanelConstructor, detailsSidebarPanelConstructors, disableBackForward);

this._canvasCollection = null;
this._canvasCollection = new WI.CanvasCollection;

this._canvasTreeOutline = new WI.TreeOutline;
this._canvasTreeOutline.addEventListener(WI.TreeOutline.Event.SelectionDidChange, this._canvasTreeOutlineSelectionDidChange, this);
@@ -143,51 +143,73 @@ WI.CanvasTabContentView = class CanvasTabContentView extends WI.ContentBrowserTa
{
super.attached();

WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasWasAdded, this._canvasAdded, this);
WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasWasRemoved, this._canvasRemoved, this);
WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasAdded, this._handleCanvasAdded, this);
WI.canvasManager.addEventListener(WI.CanvasManager.Event.CanvasRemoved, this._handleCanvasRemoved, this);
WI.canvasManager.addEventListener(WI.CanvasManager.Event.RecordingStopped, this._recordingStopped, this);
WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);
WI.RecordingContentView.addEventListener(WI.RecordingContentView.Event.RecordingActionIndexChanged, this._recordingActionIndexChanged, this);

this._canvasCollection = new WI.CanvasCollection(WI.canvasManager.canvases);
let canvases = new Set(Array.from(this._canvasCollection.items).concat(WI.canvasManager.canvases));

for (let canvas of this._canvasCollection.items) {
this._canvasTreeOutline.appendChild(new WI.CanvasTreeElement(canvas));
if (!canvases.has(canvas))
this._removeCanvas(canvas);
}

for (let recording of canvas.recordingCollection.items)
this._recordingAdded(recording, {suppressShowRecording: true});
for (let canvas of canvases) {
if (!this._canvasCollection.items.has(canvas))
this._addCanvas(canvas);
}
}

detached()
{
WI.canvasManager.removeEventListener(null, null, this);
WI.Frame.removeEventListener(null, null, this);
WI.RecordingContentView.removeEventListener(null, null, this);

this._canvasCollection = null;

this._canvasTreeOutline.removeChildren();

super.detached();
}

// Private

_canvasAdded(event)
_addCanvas(canvas)
{
let canvas = event.data.canvas;
this._canvasTreeOutline.appendChild(new WI.CanvasTreeElement(canvas));
this._canvasCollection.add(canvas);

for (let recording of canvas.recordingCollection.items)
this._recordingAdded(recording, {suppressShowRecording: true});
}

_canvasRemoved(event)
_removeCanvas(canvas)
{
let canvas = event.data.canvas;
// Move all existing recordings for the removed canvas to be imported recordings, as the
// recording's source is no longer valid.
for (let recording of canvas.recordingCollection.items) {
recording.source = null;
recording.createDisplayName();

const subtitle = null;
this._canvasTreeOutline.appendChild(new WI.GeneralTreeElement(["recording"], recording.displayName, subtitle, recording));
}

let treeElement = this._canvasTreeOutline.findTreeElement(canvas);
console.assert(treeElement, "Missing tree element for canvas.", canvas);
this._canvasTreeOutline.removeChild(treeElement);
this._canvasCollection.remove(canvas);

let currentContentView = this.contentBrowser.currentContentView;
if (currentContentView instanceof WI.RecordingContentView && canvas.recordingCollection.items.has(currentContentView.representedObject))
this.contentBrowser.updateHierarchicalPathForCurrentContentView();
}

_handleCanvasAdded(event)
{
this._addCanvas(event.data.canvas);
}

_handleCanvasRemoved(event)
{
this._removeCanvas(event.data.canvas);
}

_canvasTreeOutlineSelectionDidChange(event)
@@ -215,14 +237,6 @@ WI.CanvasTabContentView = class CanvasTabContentView extends WI.ContentBrowserTa
console.assert(false, "Unexpected representedObject.", representedObject);
}

_mainResourceDidChange(event)
{
if (!event.target.isMainFrame())
return;

this._canvasCollection.clear();
}

_recordingStopped(event)
{
let recording = event.data.recording;

0 comments on commit 02fbb71

Please sign in to comment.