Add SPI for returning a modified request in decidePolicyForNavigationAction's decisionHandler
Created attachment 316650 [details] Patch
/Volumes/Data/EWS/WebKit/Source/WebCore/loader/PolicyChecker.cpp:130:48: error: too few arguments to function call, expected 2, have 1 continueAfterNavigationPolicy(PolicyUse);
Created attachment 316673 [details] Patch
Comment on attachment 316673 [details] Patch Attachment 316673 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4204404 Number of test failures exceeded the failure limit.
Created attachment 316674 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 316673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316673&action=review I did not review the concept but I did review the copying of ResourceRequest objects. > Source/WebCore/loader/PolicyCallback.h:56 > + void setRequest(ResourceRequest& newRequest) { m_request = newRequest; } I think this should either take a ResourceRequest&& and use WTFMove, or it should take a const ResourceRequest& and always copy. Taking a non-const reference and copying does not seem right. > Source/WebCore/loader/PolicyChecker.cpp:151 > + continueAfterNavigationPolicy(action, newRequest); I think that WTFMove here would avoid copying the ResourceRequest. > Source/WebCore/loader/PolicyChecker.cpp:222 > + callback.setRequest(*newRequest); I think that WTFMove here would allow us to avoid copying the entire ResourceRequest. > Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:834 > + newRequest = navigationDecision.newRequest; Using WTFMove here we could avoid copying the ResourceRequest. > Source/WebKit/WebProcess/WebPage/WebFrame.cpp:256 > + function(action, newRequest); Adding WTFMove here would avoid copying the ResourceRequest.
Comment on attachment 316673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316673&action=review >> Source/WebCore/loader/PolicyCallback.h:56 >> + void setRequest(ResourceRequest& newRequest) { m_request = newRequest; } > > I think this should either take a ResourceRequest&& and use WTFMove, or it should take a const ResourceRequest& and always copy. Taking a non-const reference and copying does not seem right. I think PolicyCallback should be removed and replaced by WTF::Function and lambdas, but not in this patch.
We're not needing this any more