Bug 116127 - Add a preference that can disable the fake SYSV SHM shim
Summary: Add a preference that can disable the fake SYSV SHM shim
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-05-14 15:28 PDT by Simon Cooper
Modified: 2013-05-14 17:27 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.35 KB, patch)
2013-05-14 15:29 PDT, Simon Cooper
no flags Details | Formatted Diff | Diff
Patch (3.33 KB, patch)
2013-05-14 16:12 PDT, Simon Cooper
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.