[webkit-reviews] review denied: [Bug 71770] Web Inspector: display notification in the front-end when inspected worker terminates : [Attachment 113996] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 71770: Web Inspector: display notification in the front-end when inspected
worker terminates
https://bugs.webkit.org/show_bug.cgi?id=71770

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

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


We have a similar scenario in the remote debugging: front-end should display
"disconnected / terminated" message upon socket close event. You should wire
that event to the disconnected screen as well.

> Source/WebCore/inspector/front-end/HelpScreen.js:57
> +WebInspector.HelpScreen.visibleScreen_ = null;

Could you extract this refactoring into a separate change?

> Source/WebKit/chromium/src/WebDevToolsFrontendImpl.h:69
> +    virtual void agentDestroyed();

It might be that we disconnected from the agent, it is not necessarily
destroyed. At least in the downstream case, I would use "disconnect", not
"destroy".