[Webkit-unassigned] [Bug 175728] Web Inspector: Create experimental Layers tab

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=175728

--- Comment #17 from Ross Kirsling <ross.kirsling at sony.com> ---
(In reply to Devin Rousso from comment #16)
> The problem with _layersChangedWhileHidden is that it does unnecessary work
> while the tab isn't visible.  What I'm proposing is that you always call
> needsLayout() in shown(), which fixes issues #1 and #2, and move the
> addEventListener to shown() so that work is done while the tab is not
> visible.
> 
>     shown()
>     {
>         super.shown();
> 
>         this.needsLayout();
> 
>        
> WI.layerTreeManager.addEventListener(WI.LayerTreeManager.Event.
> LayerTreeDidChange, this._layerTreeDidChange, this);
> 
>         this._animate();
>     }
> 
>     hidden()
>     {
>        
> WI.layerTreeManager.removeEventListener(WI.LayerTreeManager.Event.
> LayerTreeDidChange, this._layerTreeDidChange, this);
> 
>         this._stopAnimation();
> 
>         super.hidden();
>     }
> 
> I am still not very convinced that having layersForNode() in layout() is the
> right way to go here.  I understand that it lets us avoid duplicate logic,
> but I also think that it's a bit of a hack and isn't being used properly.

I certainly did try that before reluctantly adding the flag. :)

I believe the issue was that needsLayout() gets drowned out by this early out:
https://trac.webkit.org/browser/webkit/trunk/Source/WebInspectorUI/UserInterface/Views/View.js#L263

So I had to switch it to updateLayout(), at which point I figured the _layersChangedWhileHidden approach would be wise to avoid repulling layer data when switching tabs on a static page. The event listener may apply when the tab is hidden, but the only work it does is manipulating a flag.

I forgot to mention here Issue #3, which is that hidden() gets called twice for some reason when a tab is closed. This means that removing a specific event listener from the ContentView's hidden() hook results in an assert being hit. (One workaround is to do removeEventListener(null, null, true) but this seems like quite a cop-out.)

If you have a fundamentally different approach, I'm all ears; I've certainly spent quite a few hours beating my head against this one. :P But I don't really know that it's a "hack" per se? We're still laying out objects on screen, even though they're not DOM elements, and it's very convenient to be able to use the scheduler... I am the newbie here though, so my judgments are only based on the code that I've attempted to mimic. :D

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170823/f57d03fd/attachment.html>