Bug 190525 – Web Inspector: Canvas: previews aren't removed when the parent view is hidden

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

* STEPS TO REPRODUCE:
1. Inspect <https://devinrousso.com/demo/WebKit/canvas.html>
2. Go to the Canvas tab
3. Refresh the page
4. Go to any other tab
5. Refresh the page
6. Go back to the Canvas tab
 => A Canvas preview with "No Preview Available" is shown for each canvas that was on the page before it was refreshed

[Devin Rousso]

Created attachment 352231 [details]
Patch

[Matt Baker]

Comment on attachment 352231 [details]
Patch

r=me

[Joseph Pecoraro]

Comment on attachment 352231 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/CanvasTabContentView.js:138
>          for (let canvas of this._canvasCollection) {
> -            if (!canvases.has(canvas))
> +            if (!WI.canvasManager.canvases.includes(canvas))
>                  this._removeCanvas(canvas);
>          }
>  
> -        for (let canvas of canvases) {
> +        for (let canvas of WI.canvasManager.canvases) {
>              if (!this._canvasCollection.has(canvas))
>                  this._addCanvas(canvas);
>          }

Given that `WI.canvasManager.canvases` is a non-simple getter we should only run it once:

    let canvases = WI.canvasManager.canvases;

And use this everywhere instead of `WI.canvasManager.canvases` everywhere.

[Devin Rousso]

Created attachment 352236 [details]
Patch

[Matt Baker]

(In reply to Joseph Pecoraro from comment #3)
> Comment on attachment 352231 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=352231&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/CanvasTabContentView.js:138
> >          for (let canvas of this._canvasCollection) {
> > -            if (!canvases.has(canvas))
> > +            if (!WI.canvasManager.canvases.includes(canvas))
> >                  this._removeCanvas(canvas);
> >          }
> >  
> > -        for (let canvas of canvases) {
> > +        for (let canvas of WI.canvasManager.canvases) {
> >              if (!this._canvasCollection.has(canvas))
> >                  this._addCanvas(canvas);
> >          }
> 
> Given that `WI.canvasManager.canvases` is a non-simple getter we should only
> run it once:
> 
>     let canvases = WI.canvasManager.canvases;
> 
> And use this everywhere instead of `WI.canvasManager.canvases` everywhere.

Good eye! I didn't think to check the getter.

[WebKit Commit Bot]

Comment on attachment 352236 [details]
Patch

Clearing flags on attachment: 352236

Committed r237090: <https://trac.webkit.org/changeset/237090>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45241822>