Bug 205021 - Move WKWebView code related to testing to new files, with their own headers, and stop exposing test-only functions as SPI.
Summary: Move WKWebView code related to testing to new files, with their own headers, ...
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: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on: 205133
Blocks:
  Show dependency treegraph
 
Reported: 2019-12-09 10:51 PST by Simon Fraser (smfr)
Modified: 2019-12-15 10:00 PST (History)
10 users (show)

See Also:


Attachments
Patch (147.09 KB, patch)
2019-12-09 10:52 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (147.13 KB, patch)
2019-12-09 10:55 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (138.39 KB, patch)
2019-12-09 22:15 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (138.42 KB, patch)
2019-12-10 08:23 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (298.28 KB, patch)
2019-12-10 14:51 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (299.79 KB, patch)
2019-12-10 16:23 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (296.16 KB, patch)
2019-12-12 15:00 PST, Simon Fraser (smfr)
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 Fraser (smfr) 2019-12-09 10:51:04 PST
Move WKWebView code related to testing to new files, with their own headers
Comment 1 Simon Fraser (smfr) 2019-12-09 10:52:02 PST
Created attachment 385168 [details]
Patch
Comment 2 Simon Fraser (smfr) 2019-12-09 10:55:54 PST
Created attachment 385169 [details]
Patch
Comment 3 Simon Fraser (smfr) 2019-12-09 22:15:30 PST
Created attachment 385240 [details]
Patch
Comment 4 Simon Fraser (smfr) 2019-12-10 08:23:20 PST
Created attachment 385265 [details]
Patch
Comment 5 David Quesada 2019-12-10 08:42:21 PST
Comment on attachment 385240 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:-480
> -- (void)didEndFormControlInteraction WK_API_AVAILABLE(ios(10.3));

It seems Safari is overriding these methods. Could they stay in the private header rather than being demoted to test-only SPI?

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:-576
> -@property (nonatomic, readonly) _WKFrameHandle *_mainFrame WK_API_AVAILABLE(macos(10.14.4), ios(12.2));

_hasInspectorFrontend, _inspector, and _mainFrame are used by Safari, and it doesn't seem like they were added explicitly for test purposes. Can they stay in the private header?

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:-580
> -- (void)_completeTextManipulation:(_WKTextManipulationItem *)item completion:(void(^)(BOOL success))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));

I don't think these text manipulation SPIs are test-only. (It looks like they are needed for rdar://problem/56049043) They probably shouldn't have been added to the WKTesting section in the first place.
Comment 6 Simon Fraser (smfr) 2019-12-10 09:15:30 PST
(In reply to David Quesada from comment #5)
> Comment on attachment 385240 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=385240&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:-480
> > -- (void)didEndFormControlInteraction WK_API_AVAILABLE(ios(10.3));
> 
> It seems Safari is overriding these methods. Could they stay in the private
> header rather than being demoted to test-only SPI?
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:-576
> > -@property (nonatomic, readonly) _WKFrameHandle *_mainFrame WK_API_AVAILABLE(macos(10.14.4), ios(12.2));
> 
> _hasInspectorFrontend, _inspector, and _mainFrame are used by Safari, and it
> doesn't seem like they were added explicitly for test purposes. Can they
> stay in the private header?
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:-580
> > -- (void)_completeTextManipulation:(_WKTextManipulationItem *)item completion:(void(^)(BOOL success))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> I don't think these text manipulation SPIs are test-only. (It looks like
> they are needed for rdar://problem/56049043) They probably shouldn't have
> been added to the WKTesting section in the first place.

I'll be building the Safari stack for testing, so I'll find out where Safari is using WKTesting by mistake.
Comment 7 Simon Fraser (smfr) 2019-12-10 14:51:32 PST
Created attachment 385307 [details]
Patch
Comment 8 Simon Fraser (smfr) 2019-12-10 16:23:52 PST
Created attachment 385315 [details]
Patch
Comment 9 Tim Horton 2019-12-10 16:34:03 PST
Comment on attachment 385315 [details]
Patch

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

rs=me

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.h:52
> +@property (nonatomic, readonly) BOOL _hasInspectorFrontend;

Keep properties separate!
Comment 10 Radar WebKit Bug Importer 2019-12-10 16:41:44 PST
<rdar://problem/57814555>
Comment 11 Simon Fraser (smfr) 2019-12-11 09:50:56 PST
https://trac.webkit.org/changeset/253376/webkit
Comment 12 WebKit Commit Bot 2019-12-11 13:27:46 PST
Re-opened since this is blocked by bug 205133
Comment 13 Truitt Savell 2019-12-11 14:11:24 PST
Reverted r253376 for reason:

Broke internal builds

Committed r253396: <https://trac.webkit.org/changeset/253396>
Comment 14 Simon Fraser (smfr) 2019-12-12 15:00:19 PST
Created attachment 385553 [details]
Patch
Comment 15 Simon Fraser (smfr) 2019-12-15 10:00:09 PST
Re-landed in https://trac.webkit.org/r253465 with SPI headers.