Bug 112669 – Web Inspector: Add Inspector.targetCrashed event to Inspector protocol

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

[Vladislav Kaznacheev]

Currently there is no way for a client to know if the target has crashed.

[Vladislav Kaznacheev]

Created attachment 193752 [details]
Patch

[Vladislav Kaznacheev]

This is required for https://chromiumcodereview.appspot.com/12906011/

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Yury Semikhatsky]

Comment on attachment 193752 [details]
Patch

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

> Source/WebCore/ChangeLog:7
> +

Please provide a description.

> Source/WebCore/inspector/front-end/HelpScreen.js:134
> +    WebInspector.HelpScreen.call(this, title);

I think it's time to rename HelpScreen into OverlayScreen or something as it is not actually a help screen anymore.

[Vladislav Kaznacheev]

Created attachment 193766 [details]
Patch

[Pavel Feldman]

Comment on attachment 193766 [details]
Patch

We now have a dowstream handler for that.

[Vladislav Kaznacheev]

Created attachment 193776 [details]
Patch

[Pavel Feldman]

Comment on attachment 193776 [details]
Patch

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

> Source/WebCore/inspector/Inspector.json:42
> +                "parameters": []

This is optional.

> Source/WebCore/inspector/front-end/HelpScreen.js:147
> +        WebInspector.debuggerModel.removeEventListener(WebInspector.DebuggerModel.Events.GlobalObjectCleared, this.hide, this);

I would not override this. Just call the bound function that closes and removes the listener.

> Source/WebCore/inspector/front-end/inspector.js:985
> +WebInspector.targetCrashed = function() {

{ on the next line.

> Source/WebCore/inspector/front-end/inspector.js:987
> +        WebInspector.UIString("Target has crashed"),

Please add these strings into localizedStrings.

[Vladislav Kaznacheev]

Created attachment 193783 [details]
Patch

[Vladislav Kaznacheev]

Comment on attachment 193776 [details]
Patch

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

>> Source/WebCore/inspector/Inspector.json:42
>> +                "parameters": []
> 
> This is optional.

Removed

>> Source/WebCore/inspector/front-end/HelpScreen.js:147
>> +        WebInspector.debuggerModel.removeEventListener(WebInspector.DebuggerModel.Events.GlobalObjectCleared, this.hide, this);
> 
> I would not override this. Just call the bound function that closes and removes the listener.

Discussed, agreed to do this in a separate refactoring patch (possibly disallowing manual closing of this dialog)

>> Source/WebCore/inspector/front-end/inspector.js:985
>> +WebInspector.targetCrashed = function() {
> 
> { on the next line.

Done

>> Source/WebCore/inspector/front-end/inspector.js:987
>> +        WebInspector.UIString("Target has crashed"),
> 
> Please add these strings into localizedStrings.

Done

[Pavel Feldman]

Comment on attachment 193783 [details]
Patch

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

> Source/WebCore/inspector/front-end/HelpScreen.js:135
> +    var p = this.contentElement.createChild("p");

"p help-section"

[WebKit Review Bot]

Comment on attachment 193783 [details]
Patch

Clearing flags on attachment: 193783

Committed r146198: <http://trac.webkit.org/changeset/146198>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.