Bug 229664 – add logging for IPC messages

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Cameron McCormack (:heycam)]

I have a patch.  Still wondering whether to have a single IPCMessages log channel that logs all messages, or have a separate channel per message receiver.

[Radar WebKit Bug Importer]

<rdar://problem/82511640>

[Cameron McCormack (:heycam)]

Created attachment 436758 [details]
Patch

[Cameron McCormack (:heycam)]

This is what the logging looks like, right after starting MiniBrowser:

WebProcess_InitializeWebProcess (processCreationParameters ...)
WebProcessProxy_GetNetworkProcessConnection
NetworkProcess_InitializeNetworkProcess (processCreationParameters ...)
NetworkProcess_PreconnectTo (sessionID ...) (webPageProxyID 6) (webPageID 7) (url http://192.168.20.92:8000/) (userAgent Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)) (storedCredentialsPolicy ...) (isNavigatingToAppBoundDomain ...
NetworkProcess_CreateNetworkConnectionToWebProcess (processIdentifier 8) (sessionID ...)
NetworkProcess_CreateNetworkConnectionToWebProcess Reply (connectionIdentifier ...) (cookieAcceptPolicy ...)
WebProcessProxy_GetNetworkProcessConnection Reply (connectionInfo ...)
NetworkConnectionToWebProcess_RegisterURLSchemesAsCORSEnabled (schemes [http, https])
AuxiliaryProcess_MainThreadPing
WebProcess_CreateWebPage (newPageID 7) (pageCreationParameters ...)
DrawingAreaProxy_EnterAcceleratedCompositingMode (backingStoreStateID 0) (context ...)
WebPageProxy_DidCreateMainFrame (frameID 3)
WebPageProxy_DidCreateWindow (frameID 3) (windowIdentifier ...)
WebPageProxy_DidChangeMainDocument (frameID 3)
WebProcessProxy_SetDisplayLinkPreferredFramesPerSecond (observerID 6) (displayID 0) (preferredFramesPerSecond 60)
WebProcessProxy_StartDisplayLink (observerID 6) (displayID 0) (preferredFramesPerSecond 60)
WebPageProxy_RegisterWebProcessAccessibilityToken (data ...)
WebPageProxy_FocusedFrameChanged (frameID 3)
WebProcessProxy_SetDisplayLinkPreferredFramesPerSecond (observerID 6) (displayID 0) (preferredFramesPerSecond 30)
WebProcessPool_ReportWebContentCPUTime (cpuTime 0.07s) (activityState 1)
WebPageProxy_DidChangeContentSize (newSize width=1311 height=964)
WebPageProxy_SetCanShortCircuitHorizontalWheelEvents (canShortCircuitHorizontalWheelEvents 1)
WebPageProxy_SetRenderTreeSize (treeSize 3)
WebPageProxy_SetRenderTreeSize (treeSize 3)
WebPageProxy_SampledPageTopColorChanged (sampledPageTopColor #00000000)

[Cameron McCormack (:heycam)]

Not sure if it's useful to show PIDs and/or destination IDs.

[Simon Fraser (smfr)]

Comment on attachment 436758 [details]
Patch

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

> Source/WebKit/Platform/IPC/HandleMessage.h:51
> +#ifndef NDEBUG

LOG_DISABLED

> Source/WebKit/Platform/IPC/HandleMessage.h:74
> +#ifndef NDEBUG

LOG_DISABLED

[Alex Christensen]

Source and destination pid would be quite useful.  On Darwin-based OSes, you can probably get the pid from xpc_connection_get_pid and Connection::xpcConnection(), but on other OSes you should probably leave it unimplemented.

[Cameron McCormack (:heycam)]

(In reply to Alex Christensen from comment #6)
> Source and destination pid would be quite useful.  On Darwin-based OSes, you
> can probably get the pid from xpc_connection_get_pid and
> Connection::xpcConnection(), but on other OSes you should probably leave it
> unimplemented.

Maybe I can use getCurrentProcessID() from wtf/ProcessID.h.

[Cameron McCormack (:heycam)]

(In reply to Cameron McCormack (:heycam) from comment #7)
> Maybe I can use getCurrentProcessID() from wtf/ProcessID.h.

(Oh but that doesn't get me the source PID.)

[Cameron McCormack (:heycam)]

One wrinkle is that for Connections that were created using createIPCConnectionPair() (e.g. under createNetworkConnectionToWebProcess), Connection::m_xpcConnection is null.

We only care about the remote process ID on the server-side Connection (since that's the side receiving the messages we're logging), so I guess we could pass it back through the reply messages to the Web process, if we cared to show the PIDs for messages going across those connections.

[Cameron McCormack (:heycam)]

I'm going to ignore that problem for now, and omit the remote process ID for such connections.

[Cameron McCormack (:heycam)]

Created attachment 436976 [details]
Patch

[Cameron McCormack (:heycam)]

Created attachment 437213 [details]
Patch

[Alex Christensen]

Comment on attachment 437213 [details]
Patch

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

> Source/WTF/wtf/text/TextStream.h:233
> +        if (i)

nit: I think it would be nice if this had the same style as the other two below and checked count here and used a c++11-style for ( : ) loop

> Source/WTF/wtf/text/TextStream.h:356
> +struct ValueOrFallback {

This is only ever used with "..." as the fallback.  Do we need something this generic, or could we do ValueOrEllipsis?

> Source/WebKit/Platform/IPC/HandleMessage.h:51
> +enum class ForReply { No, Yes };

nit: : bool

[Cameron McCormack (:heycam)]

Created attachment 437558 [details]
Patch

[EWS]

Committed r282115 (241412@main): <https://commits.webkit.org/241412@main>

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

[Alex Christensen]

r282126

[Fujii Hironori]

Filed: Bug 230064 – REGRESSION(r282115): undefined reference to `IPC::messageArgumentDescriptions(IPC::MessageName)' in Debug build