[webkit-reviews] review denied: [Bug 179717] Web Inspector: REGRESSION (r217750): Navigation sidebar broken after closing and re-opening tab : [Attachment 326970] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <webkit at devinrousso.com> has denied Matt Baker
<mattbaker at apple.com>'s request for review:
Bug 179717: Web Inspector: REGRESSION (r217750): Navigation sidebar broken
after closing and re-opening tab
https://bugs.webkit.org/show_bug.cgi?id=179717

Attachment 326970: Patch

https://bugs.webkit.org/attachment.cgi?id=326970&action=review




--- Comment #7 from Devin Rousso <webkit at devinrousso.com> ---
Comment on attachment 326970
  --> https://bugs.webkit.org/attachment.cgi?id=326970
Patch

r-.  The original intent of this change was twofold:
    1) ensure that each sidebar panel is only created once (really only an
issue for DetailsSidebarPanel)
    2) allow lazy instantiation of sidebar panels so that when each tab is
created, we don't need to do extra work until the tab is shown

This patch will reintroduce (2), and I don't see this as a positive change.  I
think that we should either:
    - refactor the existing NavigationSidebarPanels to use shown/hidden (moving
any addEventListener to shown) instead of closed (Debugger, Resources, Search,
and Storage)
    - allow multiple instances of the sidebars, but still instantiate them
lazily (basically have two member variables, one for the constructors and one
for the constructed SidebarPanels)

I think the first option is the more future-forward approach, as I don't think
we should ever really be doing work in a UI class that isn't visible (I like
what we do with FolderizedTreeElement), but the second option is definitely the
"faster" solution.