Preflight requests are not properly attributed as app-bound.
<rdar://problem/77664272>
Created attachment 428173 [details] Patch
Comment on attachment 428173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428173&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:410 > +- (void)_appBoundLoadData:(BOOL)isRequestingLoadsMarkedAppBound completionHandler:(void (^)(BOOL result))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); This should be two functions instead of one function with a bool indicating which function it is calling.
Comment on attachment 428173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428173&action=review > Source/WebCore/loader/CrossOriginAccessControl.cpp:92 > + preflightRequest.setIsAppBound(request.isAppBound()); I worry that other flags in the request might be missed if we rely on manually copying them over. We should take this action today, but I think we should consider a more general way to spawn a preflight request from an existing request to retain relevant state. >> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:410 >> +- (void)_appBoundLoadData:(BOOL)isRequestingLoadsMarkedAppBound completionHandler:(void (^)(BOOL result))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > This should be two functions instead of one function with a bool indicating which function it is calling. Do we need to add a separate method if these are purely for testing purposes? Passing a parameter means we could parameterize the test so the same code could be used for both flavors of test.
(In reply to Brent Fulgham from comment #4) > Comment on attachment 428173 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=428173&action=review > > > Source/WebCore/loader/CrossOriginAccessControl.cpp:92 > > + preflightRequest.setIsAppBound(request.isAppBound()); > > I worry that other flags in the request might be missed if we rely on > manually copying them over. We should take this action today, but I think we > should consider a more general way to spawn a preflight request from an > existing request to retain relevant state. Good point, I'm not exactly sure what state matters here. Marking app-bound requests is important for all network connections, but it's possible that other state isn't important for special requests like preflight checks that just check the CORS policy. > > >> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:410 > >> +- (void)_appBoundLoadData:(BOOL)isRequestingLoadsMarkedAppBound completionHandler:(void (^)(BOOL result))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); > > > > This should be two functions instead of one function with a bool indicating which function it is calling. > > Do we need to add a separate method if these are purely for testing > purposes? Passing a parameter means we could parameterize the test so the > same code could be used for both flavors of test. I see Alex's point, the test function reads weirdly with a boolean. I'm not opposed to splitting it up, but it's unfortunate that it adds so much more boilerplate WebKitTestRunner code.
Created attachment 428190 [details] Patch
Comment on attachment 428190 [details] Patch Looks good. r=me
Committed r277301 (237562@main): <https://commits.webkit.org/237562@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428190 [details].