Bug 154701 - Allow WKUserScripts to be run in isolated worlds
Summary: Allow WKUserScripts to be run in isolated worlds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-25 16:08 PST by Sam Weinig
Modified: 2016-02-26 22:10 PST (History)
2 users (show)

See Also:


Attachments
Patch (76.58 KB, patch)
2016-02-25 16:22 PST, Sam Weinig
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2016-02-25 16:08:31 PST
Allow WKUserScripts to be run in isolated worlds
Comment 1 Sam Weinig 2016-02-25 16:22:07 PST
Created attachment 272259 [details]
Patch
Comment 2 WebKit Commit Bot 2016-02-25 16:23:32 PST
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.
Comment 3 Anders Carlsson 2016-02-25 17:00:05 PST
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?
Comment 4 Sam Weinig 2016-02-26 09:50:42 PST
Committed r197172: <http://trac.webkit.org/changeset/197172>
Comment 5 Michael Catanzaro 2016-02-26 22:10:03 PST
Committed r197238: <http://trac.webkit.org/changeset/197238>