Bug 191394 – Deprecate WKBundlePageSetDefersLoading

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

Deprecate WKBundlePageSetDefersLoading

[Alex Christensen]

Created attachment 354133 [details]
Patch

[Dean Jackson]

Comment on attachment 354133 [details]
Patch

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

r=me except for the build errors.

> Source/WebKit/ChangeLog:8
> +        The last use of it was removed in <rdar://45212964>

Nit: .

[Dean Jackson]

Comment on attachment 354133 [details]
Patch

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

> Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextControllerPrivate.h:48
> +@property (nonatomic, setter=_setDefersLoading:) BOOL _defersLoading WK_API_DEPRECATED(_message, ...);

WKWebProcessPlugInBrowserContextControllerPrivate.h:48:88: error: use of undeclared identifier '_message'

[Alex Christensen]

Created attachment 361231 [details]
Patch

[EWS Watchlist]

Comment on attachment 361231 [details]
Patch

Attachment 361231 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11045604

New failing tests:
loader/load-defer.html

[EWS Watchlist]

Created attachment 361250 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 361231 [details]
Patch

Attachment 361231 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11045711

New failing tests:
loader/load-defer.html

[EWS Watchlist]

Created attachment 361256 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Alex Christensen]

Created attachment 361416 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r241127

[Radar WebKit Bug Importer]

<rdar://problem/47889372>

[Alex Christensen]

http://trac.webkit.org/r241133

[Darin Adler]

Comment on attachment 361416 [details]
Patch

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

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:746
> -void TestRunner::setDefersLoading(bool shouldDeferLoading)
> +void TestRunner::setDefersLoading(bool)
>  {
> -    WKBundlePageSetDefersLoading(InjectedBundle::singleton().page()->page(), shouldDeferLoading);
>  }

This change isn’t quite right. We should be removing this function instead of hollowing it out and leaving it. A setDefersLoading function that does nothing isn’t valuable. It’s dangerous because it lets us run tests that won’t be testing what they are trying to test.

We don’t need to keep the function to keep running the loader/load-defer.html test, because we are no longer running that test under modern WebKit, only legacy WebKit. But it’s not good that we keep running the two other tests that use it, loader/load-defer-resume-crash.html and loader/navigation-while-deferring-loads.html, because they aren’t testing what they claim to be testing any more.

I suggest we change all of those tests into legacy-WebKit-only tests and remove this function entirely.

Is that right, or am I missing something?

[Alex Christensen]

You're absolutely right, Darin.  I did so in my next patch in https://bugs.webkit.org/show_bug.cgi?id=194506