WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
179251
Make ResourceLoader::willSendRequestInternal asynchronous
https://bugs.webkit.org/show_bug.cgi?id=179251
Summary
Make ResourceLoader::willSendRequestInternal asynchronous
Alex Christensen
Reported
2017-11-03 11:34:29 PDT
Make ResourceLoader::willSendRequestInternal asynchronous
Attachments
Patch
(38.11 KB, patch)
2017-11-03 11:36 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(39.11 KB, patch)
2017-11-03 12:45 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-elcapitan
(1.03 MB, application/zip)
2017-11-03 13:25 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
(948.94 KB, application/zip)
2017-11-03 13:39 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews112 for mac-elcapitan
(2.02 MB, application/zip)
2017-11-03 13:52 PDT
,
Build Bot
no flags
Details
Patch
(40.45 KB, patch)
2017-11-03 13:54 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-elcapitan
(1.07 MB, application/zip)
2017-11-03 15:10 PDT
,
Build Bot
no flags
Details
Patch
(42.66 KB, patch)
2017-11-03 19:00 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(42.68 KB, patch)
2017-11-04 00:12 PDT
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(42.71 KB, patch)
2017-11-06 16:48 PST
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2017-11-03 11:36:53 PDT
Created
attachment 325925
[details]
Patch
Alex Christensen
Comment 2
2017-11-03 12:45:28 PDT
Created
attachment 325937
[details]
Patch
Build Bot
Comment 3
2017-11-03 13:25:41 PDT
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.
Build Bot
Comment 4
2017-11-03 13:25:42 PDT
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
Build Bot
Comment 5
2017-11-03 13:39:44 PDT
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.
Build Bot
Comment 6
2017-11-03 13:39:46 PDT
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
Build Bot
Comment 7
2017-11-03 13:52:01 PDT
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.
Build Bot
Comment 8
2017-11-03 13:52:02 PDT
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
Alex Christensen
Comment 9
2017-11-03 13:54:13 PDT
Created
attachment 325957
[details]
Patch
Build Bot
Comment 10
2017-11-03 15:10:51 PDT
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
Build Bot
Comment 11
2017-11-03 15:10:53 PDT
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
Alex Christensen
Comment 12
2017-11-03 19:00:02 PDT
Created
attachment 326000
[details]
Patch
Alex Christensen
Comment 13
2017-11-04 00:12:44 PDT
Created
attachment 326022
[details]
Patch
Andy Estes
Comment 14
2017-11-06 16:32:15 PST
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.
Alex Christensen
Comment 15
2017-11-06 16:47:45 PST
I added some protectedThis, but classes that inherit from LoaderStrategy are unfortunately process-global and never destroyed. I didn't protect them.
Alex Christensen
Comment 16
2017-11-06 16:48:10 PST
Created
attachment 326169
[details]
Patch
Alex Christensen
Comment 17
2017-11-06 16:56:02 PST
http://trac.webkit.org/r224522
Radar WebKit Bug Importer
Comment 18
2017-11-15 12:16:54 PST
<
rdar://problem/35567192
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug