Bug 71770 – Web Inspector: display notification in the front-end when inspected worker terminates

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Yury Semikhatsky]

When inspected worker terminates we should display a notification to let the user know that the worker has gone.

[Yury Semikhatsky]

Created attachment 113996 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Pavel Feldman]

Comment on attachment 113996 [details]
Patch

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".

[Yury Semikhatsky]

Comment on attachment 113996 [details]
Patch

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

>> 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".

If we need to listen to regular disconnect event there should be a separate method. This one is only called when inspected instance is actually destroyed.

[Darin Fisher (:fishd, Google)]

(In reply to comment #2)
> Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

I defer to pfeldman's review here.

[Yury Semikhatsky]

Created attachment 114038 [details]
Patch

[Pavel Feldman]

Comment on attachment 114038 [details]
Patch

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

> Source/WebCore/inspector/front-end/inspector.js:464
> +    WebInspector.log("disconnectFromBackend");

Please remove this trace.

> Source/WebCore/inspector/front-end/inspector.js:857
> +    WebInspector.log("frontendReused");

ditto

> Source/WebKit/chromium/public/WebDevToolsAgent.h:95
> +    WEBKIT_EXPORT static WebString createFrontendDisconnectedEvent();

disconnectEventAsText() ?

[Yury Semikhatsky]

Created attachment 114041 [details]
Patch for landing

All comments addressed.

[Yury Semikhatsky]

Committed r99554: <http://trac.webkit.org/changeset/99554>