Bug 182066 – Add logging to facilitate binding of WebContent and Network processes to UI process

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Keith Rollin]

When examining sysdiagnose logs and tracing events from one process to another, it would be helpful to know which WebKit processes were related to each other. When Safari, Mail, Messages, etc. are all running at the same time, it may otherwise be difficult to know if a particular Network process, for example was associated with Safari or some other application. Add some logging to the creation of WebContent and Network processes to identify their "presenting process" (parent application).

[Keith Rollin]

Created attachment 332210 [details]
Patch

[Michael Catanzaro]

Comment on attachment 332210 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:263
> +    RELEASE_LOG(Process, "%p - NetworkProcess::initializeNetworkProcess: Presenting process = %lu", this, static_cast<unsigned long>(WebCore::presentingApplicationPID()));

Seems like a good idea, but why are you casting from int to unsigned long and using %lu, instead of just using %d? I know it's weird that WebCore::presentingApplicationPID uses int rather than pid_t or unsigned int... but that really is its type.

[Brent Fulgham]

Comment on attachment 332210 [details]
Patch

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

r=me if you remove the cast and just use %d.

>> Source/WebKit/NetworkProcess/NetworkProcess.cpp:263
>> +    RELEASE_LOG(Process, "%p - NetworkProcess::initializeNetworkProcess: Presenting process = %lu", this, static_cast<unsigned long>(WebCore::presentingApplicationPID()));
> 
> Seems like a good idea, but why are you casting from int to unsigned long and using %lu, instead of just using %d? I know it's weird that WebCore::presentingApplicationPID uses int rather than pid_t or unsigned int... but that really is its type.

That is weird! Especially considering that it is a pid_t in many places. Maybe Andy can tell us why it's just an int in this method. But for this logging purpose, int seems reasonable.

[Keith Rollin]

(In reply to Brent Fulgham from comment #3)
> > Seems like a good idea, but why are you casting from int to unsigned long and using %lu, instead of just using %d? I know it's weird that WebCore::presentingApplicationPID uses int rather than pid_t or unsigned int... but that really is its type.
> 
> That is weird! Especially considering that it is a pid_t in many places.
> Maybe Andy can tell us why it's just an int in this method. But for this
> logging purpose, int seems reasonable.

I was looking into how to get a PID in WebKit. I had drilled down into RuntimeApplicationChecks.cpp/.h, which called into ProcessID.h. The base type in ProcessID.h is ProcessID, which is an int on Windows but a pid_t elsewhere. I didn't know what type a pid_t was, so I decided to promote it to something that should hold just about any value.

I neglected to notice that presentingApplicationPID in RuntimeApplicationChecks silently converts the ProcessID to an int. So making use of that information to remove my cast makes sense.

[Keith Rollin]

Created attachment 332303 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 332303 [details]
Patch

Clearing flags on attachment: 332303

Committed r227627: <https://trac.webkit.org/changeset/227627>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36881094>