Bug 154701 – Allow WKUserScripts to be run in isolated worlds

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

[Sam Weinig]

Allow WKUserScripts to be run in isolated worlds

[Sam Weinig]

Created attachment 272259 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/WebProcess/UserContent/WebUserContentController.cpp:104:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/_WKUserContentWorld.mm:83:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKit2Cocoa/_WKUserContentWorld.mm:122:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 30 files


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

[Anders Carlsson]

Comment on attachment 272259 [details]
Patch

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

> Source/WebKit2/UIProcess/API/APIUserContentWorld.cpp:61
> +    , m_name(ASCIILiteral("Normal World"))

Should normal worlds even have a name?

> Source/WebKit2/UIProcess/API/APIUserContentWorld.h:56
> +

Extra newline.

> Source/WebKit2/UIProcess/API/Cocoa/WKUserScript.mm:90
> +- (instancetype)_initWithSource:(NSString *)source injectionTime:(WKUserScriptInjectionTime)injectionTime forMainFrameOnly:(BOOL)forMainFrameOnly legacyWhitelist:(NSArray<NSString *> *)legacyWhitelist legacyBlacklist:(NSArray<NSString *> *)legacyBlacklist userContentWorld:(_WKUserContentWorld *)userContentWorld

We should really make sure that we don't ship with these legacy lists :|

> Source/WebKit2/UIProcess/API/Cocoa/_WKUserContentWorld.h:38
> +@property (nonatomic, readonly, copy) NSString *name;

Maybe name should be nullable and return nil for the normal world.

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleScriptWorld.h:42
> +    static Ref<InjectedBundleScriptWorld> create(const String& = String());

Instead of doing this, I suggest you fix the call site(s) to pass in a unique string (how about a uuid or something).

> Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInScriptWorld.h:38
> +@property (nonatomic, readonly, copy) NSString *name;

Again, I think we should make this nullable and use null for the normal world.

> Source/WebKit2/WebProcess/UserContent/WebUserContentController.cpp:100
> +    for (auto world : worlds) {

auto& here so you won't end up copying the pair.

> Source/WebKit2/WebProcess/UserContent/WebUserContentController.cpp:104
> +        worldMap().ensure(world.first, [world] { return std::make_pair(InjectedBundleScriptWorld::create(world.second), 1); });

Nice. I'd use [&] instead of [world] so you don't end up copying the pair (again).

> Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:-24
> -		1AAD19F71C7CE20300831E47 /* Coding.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1AAD19F51C7CE20300831E47 /* Coding.mm */; };

What?

> Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:-494
> -		1AAD19F51C7CE20300831E47 /* Coding.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = Coding.mm; sourceTree = "<group>"; };

What?

> Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:964
>  				A14FC5891B89927100D107EB /* ContentFilteringPlugIn.mm */,

What?

> Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:-1869
> -				1AAD19F71C7CE20300831E47 /* Coding.mm in Sources */,

What?

[Sam Weinig]

Committed r197172: <http://trac.webkit.org/changeset/197172>

[Michael Catanzaro]

Committed r197238: <http://trac.webkit.org/changeset/197238>