| Summary: | Deprecate WKBundlePageSetDefersLoading | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||||||
| Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | darin, dino, ews-watchlist, rniwa, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Alex Christensen
2018-11-07 12:53:37 PST
Created attachment 354133 [details]
Patch
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: . 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' Created attachment 361231 [details]
Patch
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 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
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 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
Created attachment 361416 [details]
Patch
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? You're absolutely right, Darin. I did so in my next patch in https://bugs.webkit.org/show_bug.cgi?id=194506 |