Bug 161502 – Web Inspector: If inspector process crashes, re-inspecting the page does not work

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Summary:
If inspector process crashes, re-inspecting the page does not work immediately.

Steps to Reproduce:
1. Inspect this page
2. shell> kill -9 <pid-of-InspectorProcess>
3. Inspect this page
  => Nothing
4. Inspect this page again
  => Inspector shows but is broken

Notes:
- WebInspectorProxy has code to handle webProcessDidCrash. It must not be cleaning up everything.

[Radar WebKit Bug Importer]

<rdar://problem/28120368>

[Joseph Pecoraro]

This happens only on Mac platforms, because WebInspectorProxyMac's platformDidClose doesn't immediately close the WebPage that crashed. It just sets a timer to close it in 60 seconds. We need a stronger "close because it crashed" case.

[Joseph Pecoraro]

Created attachment 287720 [details]
[PATCH] Proposed Fix

[Blaze Burg]

Comment on attachment 287720 [details]
[PATCH] Proposed Fix

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

r=me but please re-run through EWS when our lab is not on fire

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:171
> +    if (m_inspectorProxyObjCAdapter) {

It would be kind of worrying if this were nil. Is this possible if the process crashes prior to creating the adapter (like right after we spawn it i guess) ?

[Joseph Pecoraro]

Well this won't apply until bug 161515. I'll just wait for that.

[Joseph Pecoraro]

Comment on attachment 287720 [details]
[PATCH] Proposed Fix

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

>> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:171
>> +    if (m_inspectorProxyObjCAdapter) {
> 
> It would be kind of worrying if this were nil. Is this possible if the process crashes prior to creating the adapter (like right after we spawn it i guess) ?

I don't think it is nil, but its much easier to reason about this method when it just does the right thing regardless of the state of members.

[Joseph Pecoraro]

(In reply to comment #6)
> Comment on attachment 287720 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=287720&action=review
> 
> >> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:171
> >> +    if (m_inspectorProxyObjCAdapter) {
> > 
> > It would be kind of worrying if this were nil. Is this possible if the process crashes prior to creating the adapter (like right after we spawn it i guess) ?
> 
> I don't think it is nil, but its much easier to reason about this method
> when it just does the right thing regardless of the state of members.

I mean to say, I made these changes not because they were required, but because it just made more sense to me. Just cleanup

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/205370>