Bug 188701

Created attachment 347369 [details] Patch

Comment on attachment 347369 [details] Patch Attachment 347369 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8893390 New failing tests: fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download.html fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html

Created attachment 347379 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

Created attachment 347385 [details] Patch

Comment on attachment 347385 [details] Patch Attachment 347385 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8901361 New failing tests: imported/blink/transitions/unprefixed-transform.html

Created attachment 347443 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Comment on attachment 347443 [details] Archive of layout-test-results from ews206 for win-future Windows failure unrelated.

Comment on attachment 347385 [details] Patch Good. View in context: https://bugs.webkit.org/attachment.cgi?id=347385&action=review > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:462 > - return ShouldContinueDidReceiveResponse::Yes; > + return completionHandler(PolicyAction::Use); You're relying on the return type of the completion handler being void. Was this an accident? Or return completionHandler() a sort of way to indicate finality? I don't like it myself... I would write: completionHandler(PolicyAction::Use); return; > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:466 > - return ShouldContinueDidReceiveResponse::No; > + return completionHandler(PolicyAction::Ignore); Ditto. > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:476 > - return ShouldContinueDidReceiveResponse::No; > + return completionHandler(PolicyAction::Ignore); Ditto. > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:483 > - return ShouldContinueDidReceiveResponse::Yes; > + return completionHandler(PolicyAction::Use); Ditto.

<rdar://problem/43778059>

(In reply to Alex Christensen from comment #9) > http://trac.webkit.org/r235413 Two service worker tests are failing one of the assertions added in this change: ASSERTION FAILED: action == PolicyAction::Use /Volumes/Data/slave/highsierra-debug/build/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp(645) : auto WebKit::NetworkResourceLoader::didFinishWithRedirectResponse(WebCore::ResourceResponse &&)::(anonymous class)::operator()(type-parameter-0-0) const 1 0x1165e8269 WTFCrash 2 0x10f3805cb WTFCrashWithInfo(int, char const*, char const*, int) 3 0x10f6b011f auto WebKit::NetworkResourceLoader::didFinishWithRedirectResponse(WebCore::ResourceResponse&&)::$_22::operator()<WebCore::PolicyAction>(WebCore::PolicyAction) const 4 0x10f6b009a WTF::Function<void (WebCore::PolicyAction)>::CallableWrapper<WebKit::NetworkResourceLoader::didFinishWithRedirectResponse(WebCore::ResourceResponse&&)::$_22>::call(WebCore::PolicyAction) 5 0x10f635973 WTF::Function<void (WebCore::PolicyAction)>::operator()(WebCore::PolicyAction) const 6 0x10f616b65 WTF::CompletionHandler<void (WebCore::PolicyAction)>::operator()(WebCore::PolicyAction) 7 0x10f6639cc WebKit::NetworkResourceLoader::~NetworkResourceLoader() https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r235420%20(4647)/results.html

Re-opened since this is blocked by bug 189163

Comment on attachment 347385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347385&action=review > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:718 > - if (m_cacheEntryWaitingForContinueDidReceiveResponse) { > - continueProcessingCachedEntryAfterDidReceiveResponse(WTFMove(m_cacheEntryWaitingForContinueDidReceiveResponse)); > - return; > - } > + if (m_responseCompletionHandler) > + m_responseCompletionHandler(PolicyAction::Use); I had switched the order of these callbacks. Calling m_responseCompletionHandler (previously NetworkLoad::continueDidReceiveResponse) sometimes deletes the NetworkResourceLoader, so we can't read from its member variables later in the function.

Created attachment 350701 [details] Patch