Preflight OPTIONS requests are not shown on network panel for asynchronous XHRs.
Chromium bug: http://crbug.com/71361
Created attachment 99308 [details] Suggested patch
Comment on attachment 99308 [details] Suggested patch View in context: https://bugs.webkit.org/attachment.cgi?id=99308&action=review r- for boolean arguments, and for lack of ChangeLog explanation. I may have further comments once there is a better ChangeLog, and I actually understand the fix. > Source/WebCore/ChangeLog:7 > + Web Inspector: Preflight OPTIONS requests are not shown on network panel for asynchronous XHRs. > + https://bugs.webkit.org/show_bug.cgi?id=63712 > + ChangeLog needs an explanation of what the fix is. I can't easily understand that from the patch. > Source/WebCore/loader/MainResourceLoader.cpp:94 > + frameLoader()->notifier()->didFailToLoad(true, this, error); This is the reason why we prefer named enum values for such arguments - "true" and "false" at call sites don't tell you what they mean. Also, why is this the first argument? Is it somehow the most important one? > Source/WebCore/loader/ResourceLoadNotifier.cpp:127 > + if (!request.isNull() && oldRequestURL != request.url().string().impl()) This is nonsense code (I see that you are not adding it, but nonetheless). One should never compare StringImpls by pointer, that doesn't mean anything.
Created attachment 99462 [details] Patch with different approach
After discussion on IRC I rewrote this patch. I just added InspectorInstrumentation calls to DocumentThreadableLoader.
Comment on attachment 99462 [details] Patch with different approach View in context: https://bugs.webkit.org/attachment.cgi?id=99462&action=review Makes sense to me. Would you be willing to file a bug and possibly make a patch for incorrect StringImpl usage? > LayoutTests/http/tests/inspector/network-preflight-options.html:46 > + //if (resource.requestMethod === "POST" && !/\?deny=yes/.test(resource.url)) > + //InspectorTest.completeTest(); Commented out code? > Source/WebCore/loader/DocumentThreadableLoader.cpp:266 > + // m_actualRequest might have been reset to null in preflightFailure(); > + if (m_preflightRequestIdentifier) > + InspectorInstrumentation::didFinishLoading(m_document->frame(), m_document->frame()->loader()->documentLoader(), m_preflightRequestIdentifier, finishTime); How does this comment relate to code? > Source/WebCore/loader/DocumentThreadableLoader.cpp:362 > + // Because willSendRequest only gets called during redirects, we initialize > + // the identifier and the first willSendRequest here. No need to wrap this line - there are longer lines just below it.
Comment on attachment 99462 [details] Patch with different approach Still no useful information in ChangeLog. You should add it.
> Makes sense to me. Would you be willing to file a bug and possibly make a patch for incorrect StringImpl usage? I filed https://bugs.webkit.org/show_bug.cgi?id=63873 and will upload patch soon.
Created attachment 99572 [details] Patchi with fixes
Comment on attachment 99572 [details] Patchi with fixes Clearing flags on attachment: 99572 Committed r90340: <http://trac.webkit.org/changeset/90340>
All reviewed patches have been landed. Closing bug.