While reviewing logging we have found that some ResourceRequest objects are missing the correct state for app-initiated loads. Typically, these are cases where a new ResourceRequest is created from a URL string, losing the context that came with the original load (or the context of the page triggering the load). This patch corrects a number of such cases.
<rdar://problem/86128638>
Created attachment 446382 [details] WIP Patch
Comment on attachment 446382 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446382&action=review Thanks for covering these cases. r=me with gtk/wpe/wincairo/win fixes. > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:97 > + if (auto* documentLoader = m_document->loader()) Seems like you may need to #include DocumentLoader.h here for GTK and WPE. > Source/WebCore/loader/FrameLoader.cpp:3058 > + workingResourceRequest.setIsAppInitiated(inRequest.isAppInitiated()); We set this in updateRequestAndAddExtraFields which is called below, so no need to do it here.
Comment on attachment 446382 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446382&action=review >> Source/WebCore/Modules/websockets/WebSocketChannel.cpp:97 >> + if (auto* documentLoader = m_document->loader()) > > Seems like you may need to #include DocumentLoader.h here for GTK and WPE. Will do! >> Source/WebCore/loader/FrameLoader.cpp:3058 >> + workingResourceRequest.setIsAppInitiated(inRequest.isAppInitiated()); > > We set this in updateRequestAndAddExtraFields which is called below, so no need to do it here. Ah! Good catch. Will remove.
Created attachment 446391 [details] Patch
Created attachment 447671 [details] WIP Patch
Comment on attachment 447671 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447671&action=review > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:239 > + WTFReportBacktraceWithPrefix("WebLoaderStrategy"); This debugging code should not be landed.
Created attachment 447731 [details] Patch
Comment on attachment 447731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447731&action=review One thing to note is only the main frame's document loader holds the correct value, so there may be some places where we need to check that value instead of the current document's loader. > Source/WebCore/loader/FrameLoader.cpp:978 > + frameLoadRequest.resourceRequest().setIsAppInitiated(*appInitiated); I think this one is unnecessary because loadURL() called below will call updateRequestAndAddExtraFields which will set the value. > Source/WebKit/NetworkProcess/cache/NetworkCacheSubresourcesEntry.cpp:50 > + encoder << m_isAppInitiated; Good catch.
Comment on attachment 447731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447731&action=review > Source/WebCore/loader/NavigationScheduler.cpp:198 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); I think this will also always end up in a frame loader function that calls updateRequestAndAddExtraFields, so it is not needed. > Source/WebCore/loader/NavigationScheduler.cpp:232 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); Ditto > Source/WebCore/loader/NavigationScheduler.cpp:261 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); Ditto. > Source/WebCore/loader/NavigationScheduler.cpp:378 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); Ditto. > Source/WebCore/loader/NavigationScheduler.cpp:482 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); Ditto. > Source/WebCore/page/DOMWindow.cpp:2545 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); Ditto here. changeLocation calls loadFrameRequest which calls loadURL or loadPostRequest which will both eventually call updateRequestAndAddExtraFields. > Source/WebCore/page/DragController.cpp:300 > + resourceRequest.setIsAppInitiated(documentLoader->lastNavigationWasAppInitiated()); Here too.
Created attachment 447742 [details] Patch
Created attachment 447763 [details] Patch
Committed r287348 (245490@main): <https://commits.webkit.org/245490@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447763 [details].