Bug 45564 - Set the visible name for the web process
Summary: Set the visible name for the web process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-09-10 14:04 PDT by Anders Carlsson
Modified: 2010-09-10 15:53 PDT (History)
1 user (show)

See Also:


Attachments
Patch (4.44 KB, patch)
2010-09-10 14:07 PDT, Anders Carlsson
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2010-09-10 14:04:59 PDT
Set the visible name for the web process
Comment 1 Anders Carlsson 2010-09-10 14:07:25 PDT
<rdar://problem/8416970>
Comment 2 Anders Carlsson 2010-09-10 14:07:58 PDT
Created attachment 67232 [details]
Patch
Comment 3 Mark Rowe (bdash) 2010-09-10 14:29:44 PDT
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
Comment 4 Darin Adler 2010-09-10 14:41:49 PDT
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?
Comment 5 Anders Carlsson 2010-09-10 14:50:50 PDT
(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.
Comment 6 Anders Carlsson 2010-09-10 15:53:14 PDT
Committed r67247: <http://trac.webkit.org/changeset/67247>