[webkit-reviews] review denied: [Bug 93444] Web Inspector: SourceFrame shouldn't be a View : [Attachment 157138] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Jan Keromnes
<janx at chromium.org>'s request for review:
Bug 93444: Web Inspector: SourceFrame shouldn't be a View
https://bugs.webkit.org/show_bug.cgi?id=93444

Attachment 157138: Patch
https://bugs.webkit.org/attachment.cgi?id=157138&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=157138&action=review


> Source/WebCore/inspector/front-end/SourceFrame.js:88
> +    show: function(parent)

The change in the behavior here is that wasShown was called every time the view
was shown (every time it got on screen be it switching on the container panel
or switching to the tab that contains it). Now it happens only upon explicit
show call.

> Source/WebCore/inspector/front-end/SourceFrame.js:97
> +	   //WebInspector.View.prototype.willHide.call(this);

Same is true for detach. So I think that might be the source of the flakiness
you were seeing. The right way of addressing it would be to add a listener for
WasShown / WasHidden on the view object. Then SourceFrame would receive it
exactly when it used to and no changes to the behavior would happen. And your
show and detach would delegate to the corresponsing show and detach of the
textEditor inside.