DocumentLoader::loadMainResource() should WTFMove() the passed ResourceRequest.
Created attachment 338805 [details] Patch
Comment on attachment 338805 [details] Patch Attachment 338805 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7460742 Number of test failures exceeded the failure limit.
Created attachment 338817 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 338805 [details] Patch Attachment 338805 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/7461159 Number of test failures exceeded the failure limit.
Created attachment 338832 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 338805 [details] Patch Attachment 338805 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7461766 Number of test failures exceeded the failure limit.
Created attachment 338836 [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
Comment on attachment 338805 [details] Patch Attachment 338805 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7461926 Number of test failures exceeded the failure limit.
Created attachment 338839 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 338805 [details] Patch Attachment 338805 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7462864 Number of test failures exceeded the failure limit.
Created attachment 338847 [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 338805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338805&action=review > Source/WebCore/loader/DocumentLoader.cpp:1795 > + ResourceRequest updatedRequest; > if (mainResourceLoader()) > - request = mainResourceLoader()->originalRequest(); > + updatedRequest = mainResourceLoader()->originalRequest(); Isn't this a change in behavior if mainResourceLoader() is null?
(In reply to Alex Christensen from comment #12) > Comment on attachment 338805 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=338805&action=review > > > Source/WebCore/loader/DocumentLoader.cpp:1795 > > + ResourceRequest updatedRequest; > > if (mainResourceLoader()) > > - request = mainResourceLoader()->originalRequest(); > > + updatedRequest = mainResourceLoader()->originalRequest(); > > Isn't this a change in behavior if mainResourceLoader() is null? Yes, it is a change in behavior hence the failures. This is unintentional. Will change this snippet to read: - ResourceRequest updatedRequest; - if (mainResourceLoader()) - updatedRequest = mainResourceLoader()->originalRequest(); + ResourceRequest updatedRequest = mainResourceLoader() ? mainResourceLoader()->originalRequest() : mainResourceRequest.resourceRequest();
Committed r231052: <https://trac.webkit.org/changeset/231052>
This change caused http/tests/security/credentials-main-resource.html to fail on iOS and macOS: https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r231061%20(4939)/results.html
Re-opened since this is blocked by bug 185044
(In reply to Ryan Haddad from comment #15) > This change caused http/tests/security/credentials-main-resource.html to > fail on iOS and macOS: > https://build.webkit.org/results/ > Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r231061%20(4939)/results.html For historical preservation, here is the diff: [[ --- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/http/tests/security/credentials-main-resource-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/http/tests/security/credentials-main-resource-actual.txt @@ -1,3 +1,4 @@ -ALERT: Authenticated as user: testuser password: testpass +http://127.0.0.1:8000/security/resources/basic-auth.php?username=testuser&password=testpass - didReceiveAuthenticationChallenge - Simulating cancelled authentication sheet +ALERT: Sent username:password of (:) which is not what was expected Main Resource Credentials: testuser, testpass ]]
The failure implies that the test depends on DocumentLoader::loadMainResource() making a copy of the passed ResourceRequest because the passed ResourceRequest is mutated.
(In reply to Daniel Bates from comment #18) > The failure implies that the test depends on > DocumentLoader::loadMainResource() making a copy of the passed > ResourceRequest because the passed ResourceRequest is mutated. Disregard this remark.
Committed r231068: <https://trac.webkit.org/changeset/231068>
Committed build fix in <https://trac.webkit.org/changeset/231071>.