Bug 160831 – Move the plug-in and WebGL blacklist code to WebCore

• Component: New Bugs
• Status: RESOLVED FIXED

[Anders Carlsson]

Move the plug-in and WebGL blacklist code to WebCore

[Anders Carlsson]

Created attachment 285971 [details]
Patch

[Anders Carlsson]

Created attachment 285975 [details]
Patch

[WebKit Commit Bot]

Attachment 285975 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mac/PluginBlacklist.mm:150:  Missing space around : in range-based for statement  [whitespace/colon] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Comment on attachment 285971 [details]
Patch

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

> Source/WebCore/platform/mac/BlacklistUpdater.h:33
> +#include <Foundation/Foundation.h>
> +#include <memory>
> +#include <string>
> +#include <vector>

I'm not sure you need any of these.  Maybe something for dispatch_queue_t.

> Source/WebCore/platform/mac/BlacklistUpdater.h:51
> +private:
> +
> +    static NSDictionary *readBlacklistData();

Extra newline.

> Source/WebCore/platform/mac/BlacklistUpdater.mm:59
> +NSDictionary * BlacklistUpdater::readBlacklistData()

Extra space.

> Source/WebCore/platform/mac/BlacklistUpdater.mm:90
> +        s_pluginBlacklist = 0;

nullptr.

> Source/WebCore/platform/mac/BlacklistUpdater.mm:95
> +        s_webGLBlacklist = 0;

nullptr.

> Source/WebCore/platform/mac/PluginBlacklist.h:66
> +    NSDictionary *m_bundleIDToMinimumSecureVersion;
> +    NSDictionary *m_bundleIDToMinimumCompatibleVersion;
> +    NSDictionary *m_bundleIDToBlockedVersions;
> +    NSSet *m_bundleIDsWithAvailableUpdates;

RetainPtrs?

> Source/WebCore/platform/mac/PluginBlacklist.mm:72
> +    CFDictionaryRef systemVersionDictionary = _CFCopySystemVersionDictionary();

RetainPtr?

> Source/WebCore/platform/mac/PluginBlacklist.mm:87
> +        for (NSString *bundleID in bundleIDs) {

What is guaranteeing these are all NSStrings?

> Source/WebCore/platform/mac/PluginBlacklist.mm:89
> +            assert(versionInfo);

ASSERT?

> Source/WebCore/platform/mac/PluginBlacklist.mm:117
> +    return std::unique_ptr<PluginBlacklist>(new PluginBlacklist(bundleIDToMinimumSecureVersion, bundleIDToMinimumCompatibleVersion, bundleIDToBlockedVersions, bundleIDsWithAvailableUpdates));

std::make_unique?

> Source/WebCore/platform/mac/WebGLBlacklist.mm:74
> +    int major; // Represents the 13 in 13C64.
> +    int minor; // Represents the C in 13C64 (as a number where A == 1, i.e. 3).
> +    int build; // Represents the 64 in 13C64.

= 0; here to avoid need for default constructor.

> Source/WebCore/platform/mac/WebGLBlacklist.mm:86
> +        return OSBuildInfo();

return { };

> Source/WebCore/platform/mac/WebGLBlacklist.mm:153
> +    unsigned maskValue;
> +    [scanner scanHexInt:&maskValue];

Please write a function that given a string with hex digits returns an unsigned 32-bit int.

> Source/WebCore/platform/mac/WebGLBlacklist.mm:171
> +    CFDictionaryRef systemVersionDictionary = _CFCopySystemVersionDictionary();

RetainPtr?

> Source/WebCore/platform/mac/WebGLBlacklist.mm:215
> +    for (NSDictionary *blockData in blockEntries) {
> +
> +        GLint gpuMask = gpuMaskFromString([blockData objectForKey:@"GPU"]);

Extra newline.

> Source/WebCore/platform/mac/WebGLBlacklist.mm:242
> +    return std::unique_ptr<WebGLBlacklist>(new WebGLBlacklist(globalCommand));

std::make_unique

> Source/WebKit2/UIProcess/Plugins/PluginInfoStore.h:32
> +#include <WebCore/PluginBlacklist.h>

Why is this change needed?

[Sam Weinig]

Comment on attachment 285975 [details]
Patch

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

> Source/WebCore/platform/mac/PluginBlacklist.mm:46
> +        PluginBlacklist* pluginBlacklist = BlacklistUpdater::pluginBlacklist();

auto*?

> Source/WebCore/platform/mac/PluginBlacklist.mm:62
> +        PluginBlacklist* pluginBlacklist = BlacklistUpdater::pluginBlacklist();

auto*

> Source/WebCore/platform/mac/WebGLBlacklist.mm:113
> +        WebGLBlacklist* webGLBlacklist = BlacklistUpdater::webGLBlacklist();

auto*

> Source/WebCore/platform/mac/WebGLBlacklist.mm:129
> +        WebGLBlacklist* webGLBlacklist = BlacklistUpdater::webGLBlacklist();

auto*

> Source/WebCore/platform/mac/WebGLBlacklist.mm:211
> +    BlockCommand globalCommand = BlockCommand::Allow;

auto?

> Source/WebCore/platform/mac/WebGLBlacklist.mm:217
> +        OSBuildInfo blockedBuildInfo = buildInfoFromOSBuildString(static_cast<NSString*>([blockData objectForKey:@"OSBuild"]));

auto?

> Source/WebCore/platform/mac/WebGLBlacklist.mm:220
> +        BlockComparison comparison = BlockComparison::Equals;

auto?

> Source/WebCore/platform/mac/WebGLBlacklist.mm:227
> +        BlockCommand command = BlockCommand::Allow;

auto?

[Anders Carlsson]

Created attachment 285980 [details]
Patch

[WebKit Commit Bot]

Attachment 285980 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mac/PluginBlacklist.mm:150:  Missing space around : in range-based for statement  [whitespace/colon] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Anders Carlsson]

(In reply to comment #5)
> Comment on attachment 285975 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285975&action=review
> 
> > Source/WebCore/platform/mac/PluginBlacklist.mm:46
> > +        PluginBlacklist* pluginBlacklist = BlacklistUpdater::pluginBlacklist();
> 
> auto*?
> 
> > Source/WebCore/platform/mac/PluginBlacklist.mm:62
> > +        PluginBlacklist* pluginBlacklist = BlacklistUpdater::pluginBlacklist();
> 
> auto*
> 
> > Source/WebCore/platform/mac/WebGLBlacklist.mm:113
> > +        WebGLBlacklist* webGLBlacklist = BlacklistUpdater::webGLBlacklist();
> 
> auto*
> 
> > Source/WebCore/platform/mac/WebGLBlacklist.mm:129
> > +        WebGLBlacklist* webGLBlacklist = BlacklistUpdater::webGLBlacklist();
> 
> auto*
> 
> > Source/WebCore/platform/mac/WebGLBlacklist.mm:211
> > +    BlockCommand globalCommand = BlockCommand::Allow;
> 
> auto?
> 
> > Source/WebCore/platform/mac/WebGLBlacklist.mm:217
> > +        OSBuildInfo blockedBuildInfo = buildInfoFromOSBuildString(static_cast<NSString*>([blockData objectForKey:@"OSBuild"]));
> 
> auto?
> 
> > Source/WebCore/platform/mac/WebGLBlacklist.mm:220
> > +        BlockComparison comparison = BlockComparison::Equals;
> 
> auto?
> 
> > Source/WebCore/platform/mac/WebGLBlacklist.mm:227
> > +        BlockCommand command = BlockCommand::Allow;
> 
> auto?

I'll resolve these in an upcoming patch - I wanted to keep the changes between WKSI and Open Source minimal.

[Anders Carlsson]

Committed r204462: <http://trac.webkit.org/changeset/204462>

[Hunseop Jeong]

CMake build fix
Committed r204548: <http://trac.webkit.org/changeset/204548>