Bug 205021 – Move WKWebView code related to testing to new files, with their own headers, and stop exposing test-only functions as SPI.

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

[Simon Fraser (smfr)]

Move WKWebView code related to testing to new files, with their own headers

[Simon Fraser (smfr)]

Created attachment 385168 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 385169 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 385240 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 385265 [details]
Patch

[David Quesada]

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.

[Simon Fraser (smfr)]

(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.

[Simon Fraser (smfr)]

Created attachment 385307 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 385315 [details]
Patch

[Tim Horton]

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!

[Radar WebKit Bug Importer]

<rdar://problem/57814555>

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/253376/webkit

[WebKit Commit Bot]

Re-opened since this is blocked by bug 205133

[Truitt Savell]

Reverted r253376 for reason:

Broke internal builds

Committed r253396: <https://trac.webkit.org/changeset/253396>

[Simon Fraser (smfr)]

Created attachment 385553 [details]
Patch

[Simon Fraser (smfr)]

Re-landed in https://trac.webkit.org/r253465 with SPI headers.