Summary: | Make ResourceLoader::willSendRequestInternal asynchronous | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||||||||||||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | aestes, buildbot, cdumez, dbates, japhet, rniwa, thorton, webkit-bug-importer | ||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=182270 | ||||||||||||||||||||||||
Attachments: |
|
Description
Alex Christensen
2017-11-03 11:34:29 PDT
Created attachment 325925 [details]
Patch
Created attachment 325937 [details]
Patch
Comment on attachment 325937 [details] Patch Attachment 325937 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5094128 Number of test failures exceeded the failure limit. Created attachment 325947 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 325937 [details] Patch Attachment 325937 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5094253 Number of test failures exceeded the failure limit. Created attachment 325951 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 325937 [details] Patch Attachment 325937 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5094263 Number of test failures exceeded the failure limit. Created attachment 325955 [details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 325957 [details]
Patch
Comment on attachment 325957 [details] Patch Attachment 325957 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5095568 New failing tests: imported/w3c/web-platform-tests/fetch/api/cors/cors-redirect-preflight.any.html Created attachment 325969 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 326000 [details]
Patch
Created attachment 326022 [details]
Patch
Comment on attachment 326022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326022&action=review > Source/WebCore/loader/SubresourceLoader.cpp:243 > + ResourceLoader::willSendRequestInternal(WTFMove(newRequest), redirectResponse, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler), redirectResponse = redirectResponse] (ResourceRequest&& request) mutable { You can just type "redirectResponse" to capture by value. You don't need to type "redirectResponse = redirectResponse". > Source/WebCore/loader/cache/CachedResource.cpp:297 > + platformStrategies()->loaderStrategy()->loadResource(frame, *this, WTFMove(request), m_options, [this, frame = makeRef(frame), loggingAllowed = cachedResourceLoader.isAlwaysOnLoggingAllowed()] (RefPtr<SubresourceLoader>&& loader) { Is capturing |this| safe here? Do you need a CachedResourceHandle? > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:93 > + SubresourceLoader::create(frame, resource, WTFMove(request), options, [this, completionHandler = WTFMove(completionHandler), resource = CachedResourceHandle<CachedResource>(&resource), frame = makeRef(frame)] (RefPtr<SubresourceLoader>&& loader) mutable { Is it safe to capture |this| here without protecting it? > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:104 > + NetscapePlugInStreamLoader::create(frame, client, WTFMove(request), [this, completionHandler = WTFMove(completionHandler), frame = makeRef(frame)] (RefPtr<NetscapePlugInStreamLoader>&& loader) mutable { Ditto. > Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.cpp:93 > + SubresourceLoader::create(frame, resource, WTFMove(request), options, [this, completionHandler = WTFMove(completionHandler)] (RefPtr<WebCore::SubresourceLoader>&& loader) mutable { Ditto. > Source/WebKitLegacy/WebCoreSupport/WebResourceLoadScheduler.cpp:116 > + NetscapePlugInStreamLoader::create(frame, client, WTFMove(request), [this, completionHandler = WTFMove(completionHandler)] (RefPtr<WebCore::NetscapePlugInStreamLoader>&& loader) mutable { Ditto. I added some protectedThis, but classes that inherit from LoaderStrategy are unfortunately process-global and never destroyed. I didn't protect them. Created attachment 326169 [details]
Patch
|