Bug 116127

Summary: Add a preference that can disable the fake SYSV SHM shim
Product: WebKit Reporter: Simon Cooper <scooper>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, scooper
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Simon Cooper 2013-05-14 15:28:24 PDT
Add a preference that can disable the fake SYSV SHM shim
Comment 1 Simon Cooper 2013-05-14 15:29:27 PDT
Created attachment 201764 [details]
Patch
Comment 2 Alexey Proskuryakov 2013-05-14 15:40:18 PDT
Comment on attachment 201764 [details]
Patch

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

Looks good, please fix style nits and upload a new patch.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:161
> +    static int cachedPrefValue = 0;

Please change cachedPrefValue to cachedPreferenceValue.

This is not one of the idioms we use in WebKit. I'm OK with this code as is, but for reference, we'd do one of these:

1.
static bool preferenceValue = getShimDisabledPreference();
return preferenceValue;

2. dispatch_once(...)

#2 is the preferred option these days.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:171
> +        if (keyExistsAndHasValidFormat && prefValue) {
> +            cachedPrefValue = 1;
> +        } else {
> +            cachedPrefValue = -1;
> +        }

Please remove braces around single line blocks.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:181
> +    if (shim_disabled()) {
> +        return shmdt(sharedAddress);
> +    }

Please remove braces around single line blocks.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:202
> +    if (shim_disabled()) {
> +        return shmat(sharedMemoryIdentifier, requestedSharedAddress, shmflg);
> +    }

Ditto.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:231
> +    if (shim_disabled()) {
> +        return shmget(key, requestedSizeOfSharedMemory, sharedMemoryFlags);
> +    }

Ditto.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:267
> +    if (shim_disabled()) {
> +        return shmctl(sharedMemoryIdentifier, cmd, outputDescriptor);
> +    }

Ditto.
Comment 3 Simon Cooper 2013-05-14 16:12:26 PDT
Created attachment 201768 [details]
Patch
Comment 4 Alexey Proskuryakov 2013-05-14 16:31:43 PDT
Comment on attachment 201768 [details]
Patch

I'm assuming that you wanted commit queue flag on this patch, too.
Comment 5 WebKit Commit Bot 2013-05-14 17:27:01 PDT
Comment on attachment 201768 [details]
Patch

Clearing flags on attachment: 201768

Committed r150094: <http://trac.webkit.org/changeset/150094>
Comment 6 WebKit Commit Bot 2013-05-14 17:27:02 PDT
All reviewed patches have been landed.  Closing bug.