Bug 108015 – Refactor XPCService initialization to make it easier to add more services

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Refactor XPCService initialization to make it easier to add more services

[Sam Weinig]

Created attachment 184887 [details]
Patch

Comment on attachment 184887 [details]
Patch

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

r=me if you fix the macro name, but feel free to address other comments.

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:30
> +#error XPC_SERVICE_INITIALIZER must be defined.

The XPC_ prefix for use by XPC. Please prefix this with something WebKit-specific.

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:63
> +                // Setup the posix_spawn attributes.

s/Setup/Set up/. What is this comment for anyway?

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:83
> +                // Set the architecture.
> +                cpu_type_t cpuTypes[] = { (cpu_type_t)xpc_dictionary_get_uint64(event, "architecture") };
> +                size_t outCount = 0;
> +                posix_spawnattr_setbinpref_np(&attr, 1, cpuTypes, &outCount);

Why is this stuck in the middle of the section that sets the flags?

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:95
> +                // Setup the command line.

s/Setup/Set up/. What is this comment for anyway?

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:100
> +                // Setup the environment.

Ditto.

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:113
> +                    size_t stringLength = strlen(string);
> +
> +                    char* environmentVariable = (char*)malloc(stringLength + 1);
> +                    memcpy(environmentVariable, string, stringLength);
> +                    environmentVariable[stringLength] = '\0';
> +
> +                    environment[i] = environmentVariable;

Isn’t this equivalent to:
    environment[i] = strdup(environmentVariable)
? Why is this required and a simple environment[i] = environmentVariable won’t do? Are the pointers in the environment parameter required to stay valid after the call to posix_spawn?

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:122
> +                NSLog(@"Unable to re-exec for path: %s\n", path);

No need for \n in NSLog.

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:129
> +                    NSLog(@"Unable to load WebKit2.framework: %s\n", dlerror());

Might be useful to log the path here.

> Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceBootstrapper.Development.h:136
> +                    NSLog(@"Unable to find entry point in WebKit2.framework: %s\n", dlerror());

Might be useful to log the name here.

[Sam Weinig]

Committed r140919: <http://trac.webkit.org/changeset/140919>