Right now there is no indication that anything has happened and no callbacks, which causes asynchronous XHR to behave strangely. Let's call onerror instead.
Created attachment 256345 [details] Patch
Comment on attachment 256345 [details] Patch Attachment 256345 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4673705569419264 New failing tests: http/tests/security/mixedContent/insecure-xhr-in-main-frame.html fast/frames/frame-unload-crash.html http/tests/navigation/subframe-pagehide-handler-starts-load.html http/tests/navigation/subframe-pagehide-handler-starts-load2.html
Created attachment 256348 [details] Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 256345 [details] Patch Attachment 256345 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4688632728256512 New failing tests: http/tests/security/mixedContent/insecure-xhr-in-main-frame.html fast/frames/frame-unload-crash.html http/tests/navigation/subframe-pagehide-handler-starts-load.html http/tests/navigation/subframe-pagehide-handler-starts-load2.html
Created attachment 256349 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 256345 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256345&action=review > Source/WebCore/xml/XMLHttpRequest.cpp:791 > + // ThreadableLoader::create can return null here, for example if we're no longer attached to a page or if a content blocker blocks the load. > // This is true while running onunload handlers. > // FIXME: Maybe we need to be able to send XMLHttpRequests from onunload, <http://bugs.webkit.org/show_bug.cgi?id=10904>. > - // FIXME: Maybe create() can return null for other reasons too? > m_loader = ThreadableLoader::create(scriptExecutionContext(), this, request, options); > if (m_loader) { > // Neither this object nor the JavaScript wrapper should be deleted while > // a request is in progress because we need to keep the listeners alive, > // and they are referenced by the JavaScript wrapper. > setPendingActivity(this); > - } > + } else > + networkError(); I hesitate to believe that synchronously calling networkError here is the right thing to do. I'd wager that it should be called on a 0-delay timer.
(In reply to comment #6) > Comment on attachment 256345 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=256345&action=review > > > Source/WebCore/xml/XMLHttpRequest.cpp:791 > > + // ThreadableLoader::create can return null here, for example if we're no longer attached to a page or if a content blocker blocks the load. > > // This is true while running onunload handlers. > > // FIXME: Maybe we need to be able to send XMLHttpRequests from onunload, <http://bugs.webkit.org/show_bug.cgi?id=10904>. > > - // FIXME: Maybe create() can return null for other reasons too? > > m_loader = ThreadableLoader::create(scriptExecutionContext(), this, request, options); > > if (m_loader) { > > // Neither this object nor the JavaScript wrapper should be deleted while > > // a request is in progress because we need to keep the listeners alive, > > // and they are referenced by the JavaScript wrapper. > > setPendingActivity(this); > > - } > > + } else > > + networkError(); > > I hesitate to believe that synchronously calling networkError here is the > right thing to do. > > I'd wager that it should be called on a 0-delay timer. Additionally, as the comment suggests, ThreadableLoader::create can fail for other reasons besides content blocking, such as when there's no page. Is it safe to dispatch events without a page?
(In reply to comment #6) > I hesitate to believe that synchronously calling networkError here is the > right thing to do. > > I'd wager that it should be called on a 0-delay timer. networkError calls dispatchErrorEvents
Created attachment 256386 [details] Patch
(In reply to comment #8) > (In reply to comment #6) > > I hesitate to believe that synchronously calling networkError here is the > > right thing to do. > > > > I'd wager that it should be called on a 0-delay timer. > networkError calls dispatchErrorEvents ...which synchronously dispatches. :)
I'm curious: is there a reason why the Content Extensions blocking check isn't done during open()? (It looks like we already do that with CSP checks). That'd remove the need for a separate error path for synchronous XHRs vs async ones (see https://bugs.webkit.org/show_bug.cgi?id=146271 for what's currently in place for sync XHR).
Bump. :)
<rdar://problem/22925459>
Created attachment 263028 [details] Patch
Comment on attachment 263028 [details] Patch XHR has onerror. This patch should make it be called.
Created attachment 263044 [details] Patch
(In reply to comment #15) > XHR has onerror. > > This patch should make it be called. It was called, it just wasn't in the test of the previous patch. It is in the latest test.
Comment on attachment 263044 [details] Patch Clearing flags on attachment: 263044 Committed r191077: <http://trac.webkit.org/changeset/191077>
All reviewed patches have been landed. Closing bug.
Tested with latest WebKit nightly (r191112) on 10.11 (15A284), and onerror doesn't seem to get called.
*** Bug 145717 has been marked as a duplicate of this bug. ***