Bug 207351 - Cleanup and promote WKContentWorld and its clients in WKWebView
Summary: Cleanup and promote WKContentWorld and its clients in WKWebView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-06 13:55 PST by Brady Eidson
Modified: 2020-02-11 09:48 PST (History)
4 users (show)

See Also:


Attachments
Patch (64.04 KB, patch)
2020-02-06 14:06 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (1.17 KB, patch)
2020-02-07 12:55 PST, Brady Eidson
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2020-02-06 13:55:46 PST
Cleanup and promote WKContentWorld and its clients in WKWebView
Comment 1 Brady Eidson 2020-02-06 14:06:36 PST
Created attachment 389993 [details]
Patch
Comment 2 Alex Christensen 2020-02-06 14:16:01 PST
Comment on attachment 389993 [details]
Patch

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

> Source/WebKit/Shared/Cocoa/APIObject.mm:-351
> -        wrapper = [_WKContentWorld alloc];

There were no clients of this old class, right?  Otherwise we would need a wrapper to keep binary compatibility.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:828
> +- (void)callAsyncJavaScript:(NSString *)javaScriptString arguments:(NSDictionary<NSString *, id> *)arguments inContentWorld:(WKContentWorld *)contentWorld completionHandler:(void (^)(id, NSError *error))completionHandler

The JavaScript is not necessarily Async.  Could we remove that?
If not, do we want to write out Asynchronous?
Comment 3 Brady Eidson 2020-02-06 14:21:44 PST
(In reply to Alex Christensen from comment #2)
> Comment on attachment 389993 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389993&action=review
> 
> > Source/WebKit/Shared/Cocoa/APIObject.mm:-351
> > -        wrapper = [_WKContentWorld alloc];
> 
> There were no clients of this old class, right?  Otherwise we would need a
> wrapper to keep binary compatibility.

No clients, correct.

> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:828
> > +- (void)callAsyncJavaScript:(NSString *)javaScriptString arguments:(NSDictionary<NSString *, id> *)arguments inContentWorld:(WKContentWorld *)contentWorld completionHandler:(void (^)(id, NSError *error))completionHandler
> 
> The JavaScript is not necessarily Async.  Could we remove that?
> If not, do we want to write out Asynchronous?

This name is what internal discussion settled on.
Comment 4 WebKit Commit Bot 2020-02-06 18:26:41 PST
Comment on attachment 389993 [details]
Patch

Clearing flags on attachment: 389993

Committed r255998: <https://trac.webkit.org/changeset/255998>
Comment 5 WebKit Commit Bot 2020-02-06 18:26:43 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2020-02-06 18:27:11 PST
<rdar://problem/59247330>
Comment 7 Alex Christensen 2020-02-07 10:39:08 PST
Comment on attachment 389993 [details]
Patch

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

> Source/WebKit/ChangeLog:17
> +        * UIProcess/API/Cocoa/WKContentWorld.h: Renamed from Source/WebKit/UIProcess/API/Cocoa/_WKContentWorld.h.

You should add this to WebKit.h.
Comment 8 Brady Eidson 2020-02-07 12:55:29 PST
Reopening to attach new patch.
Comment 9 Brady Eidson 2020-02-07 12:55:30 PST
Created attachment 390116 [details]
Patch
Comment 10 WebKit Commit Bot 2020-02-07 14:18:46 PST
Comment on attachment 390116 [details]
Patch

Rejecting attachment 390116 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 390116, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebKit/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: https://webkit-queues.webkit.org/results/13318758
Comment 11 Alex Christensen 2020-02-11 09:48:28 PST
http://trac.webkit.org/r256312