Bug 45564 – Set the visible name for the web process

• Component: WebKit2
• Status: RESOLVED FIXED

[Anders Carlsson]

Set the visible name for the web process

[Anders Carlsson]

<rdar://problem/8416970>

[Anders Carlsson]

Created attachment 67232 [details]
Patch

[Mark Rowe (bdash)]

Comment on attachment 67232 [details]
Patch

> @@ -45,6 +47,23 @@ extern "C" kern_return_t bootstrap_register2(mach_port_t, name_t, mach_port_t, u
>  
>  namespace WebKit {
>  
> +// -[NSProcessInfo processName] isn't thread-safe so we have our own implementation.

We should do this conditionally and use -[NSProcessInfo processName] on platforms where it is thread-safe.

> +static const char* processName() {

Misplaced {.

> +    static CString* processName;
> +    if (!processName) {
> +        uint32_t bufferSize = MAXPATHLEN;
> +        char executablePath[bufferSize];
> +
> +        if (_NSGetExecutablePath(executablePath, &bufferSize))
> +            return “”;

If _NSGetExecutablePath fails we’ll do this work every time the function is called.  Is that expected?  Does _NSGetExecutablePath fail temporarily and then later give us valid results?

> @@ -95,6 +96,14 @@ int WebProcessMain(CommandLine* commandLine)
>      WTF::initializeMainThread();
>      RunLoop::initializeMainRunLoop();
>  
> +    // Set the visible application name.
> +    String parentProcessName = (*commandLine)["parentprocessname"];
> +    if (!parentProcessName.isNull()) {
> +        // FIXME: Localization!
> +        NSString *applicationName = [NSString stringWithFormat:@"%@ Web Content", (NSString *)parentProcessName];
> +        WKSetVisibleApplicationName((CFStringRef)applicationName);
> +    }

It would’ve been nicer if WKSetVisibleApplicationName took an NSString*.

r=me

[Darin Adler]

Comment on attachment 67232 [details]
Patch

> +static const char* processName() {

Brace should move to the next line.

> +    static CString* processName;
> +    if (!processName) {
> +        uint32_t bufferSize = MAXPATHLEN;
> +        char executablePath[bufferSize];
> +
> +        if (_NSGetExecutablePath(executablePath, &bufferSize))
> +            return "";
> +
> +        char *name = strrchr(executablePath, '/') + 1;
> +        processName = new CString(name);
> +    }

It would be better factoring to have this in a separate function called createProcessName.

And if we want to leak this can we just strdup instead of using a CString?

> +        // FIXME: Localization!
> +        NSString *applicationName = [NSString stringWithFormat:@"%@ Web Content", (NSString *)parentProcessName];
> +        WKSetVisibleApplicationName((CFStringRef)applicationName);

That’s an admirable FIXME, but what’s your plan here?

[Anders Carlsson]

(In reply to comment #4)
> (From update of attachment 67232 [details])
> > +static const char* processName() {
> 
> Brace should move to the next line.
> 
> > +    static CString* processName;
> > +    if (!processName) {
> > +        uint32_t bufferSize = MAXPATHLEN;
> > +        char executablePath[bufferSize];
> > +
> > +        if (_NSGetExecutablePath(executablePath, &bufferSize))
> > +            return "";
> > +
> > +        char *name = strrchr(executablePath, '/') + 1;
> > +        processName = new CString(name);
> > +    }
> 
> It would be better factoring to have this in a separate function called createProcessName.
> 

Sure, I'll do that. (On platforms where NSProcessInfo is thread-safe I'll just have processName() return -[NSProcessInfo processName]).

> And if we want to leak this can we just strdup instead of using a CString?

Yes.

> 
> > +        // FIXME: Localization!
> > +        NSString *applicationName = [NSString stringWithFormat:@"%@ Web Content", (NSString *)parentProcessName];
> > +        WKSetVisibleApplicationName((CFStringRef)applicationName);
> 
> That’s an admirable FIXME, but what’s your plan here?

I don't know how to set up the localization infrastructure, but we should probably do it sooner rather than later.

[Anders Carlsson]

Committed r67247: <http://trac.webkit.org/changeset/67247>