[webkit-reviews] review granted: [Bug 181155] Add a ProcessIdentifier, vended from the UI process, to each child process : [Attachment 330209] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 181155: Add a ProcessIdentifier, vended from the UI process, to each child
process
https://bugs.webkit.org/show_bug.cgi?id=181155

Attachment 330209: Patch

https://bugs.webkit.org/attachment.cgi?id=330209&action=review




--- Comment #2 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 330209
  --> https://bugs.webkit.org/attachment.cgi?id=330209
Patch

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

Looks good! I had a few idle thoughts, but nothing you need to address before
landing. r=me.

> Source/WebCore/ChangeLog:4
> +	   https://bugs.webkit.org/show_bug.cgi?id=181155

Yes, please.

> Source/WebCore/platform/Process.cpp:46
> +    static bool hasInitialized;

Don't we usually prefer a dispatch_once for this kind of initialization? Maybe
we don't for cross-platform code.

    static dispatch_once_t onceToken;
    dispatch_once(&onceToken, ^ {
	globalIdentifier = generateObjectIdentifier<ProcessIdentifierType>();
    });

> Source/WebKit/Shared/ChildProcess.cpp:73
> +#endif

I think 'processIdentifier' is guaranteed to exist (on all platforms), since
you default initialize it in the ChildProcessProxy, and always add it to the
init parameters in getLaunchOptions.

>
Source/WebKit/Shared/EntryPointUtilities/mac/XPCService/XPCServiceEntryPoint.mm
:90
> +    if (processIdentifierString.isEmpty())

Should this assert? It would be super weird if this ever failed, wouldn't it?