Bug 236672 – Web Inspector: [Cocoa] Reentrancy in WebKit::WebInspectorUIProxy::open

• Component: Web Inspector
• Status: RESOLVED FIXED

[Patrick Angle]

<rdar://88358696>

[Patrick Angle]

Created attachment 452097 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 452097 [details]
Patch v1.0

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

rs=me

> Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm:406
> +            WTFLogAlways("WebInspectorUIProxy::platformBringToFront - Inspected and inspector windows did not match while opening inspector. Falling back to detached inspector. Inspected page had window: %s", inspectedPage()->platformWindow() ? "YES" : "NO");

Do we _always_ want/need to log this?

Also, maybe we should use the `Inspector` log channel (e.g. `LOG(Inspector, ...);`)?

[Patrick Angle]

Comment on attachment 452097 [details]
Patch v1.0

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

>> Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm:406
>> +            WTFLogAlways("WebInspectorUIProxy::platformBringToFront - Inspected and inspector windows did not match while opening inspector. Falling back to detached inspector. Inspected page had window: %s", inspectedPage()->platformWindow() ? "YES" : "NO");
> 
> Do we _always_ want/need to log this?
> 
> Also, maybe we should use the `Inspector` log channel (e.g. `LOG(Inspector, ...);`)?

I'd ideally like this logging to be present when someone files an issue with a sysdiagnose saying "hey, if I do these specific things in this order, Web Inspector detaches from the view into its own window". I can use `RELEASE_LOG` instead though, which allows us to target the Inspector channel. I think you are right that `*LOG(Inspector` is probably a better idea than `WTFLogAlways` here. Part of my thought process behind having the logging is so that we can hopefully better understand a report of "Web Inspector detached into a separate window" and get further clues as to the conditions that cause this in the first place.

[Patrick Angle]

Created attachment 452110 [details]
[fast-cq] Patch v1.1 - Use RELEASE_LOG for logging

[EWS]

Committed r289875 (247313@main): <https://commits.webkit.org/247313@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452110 [details].