<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[206524] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/206524">206524</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2016-09-28 09:37:26 -0700 (Wed, 28 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>WebCore::ResourceErrorBase::setType is crashing
https://bugs.webkit.org/show_bug.cgi?id=162484
<rdar://problem/28390828>
Patch by Youenn Fablet <youenn@apple.com> on 2016-09-28
Reviewed by Alex Christensen.
Source/WebCore:
Test: http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html
Behavior is slightly changed as we are no longer casting Timeout preflight errors as AccessControl errors.
This is more inline with fetch spec which prescribes to send back any error received by preflight as response error for fetch.
Ideally, we should not need to change errors received during preflight loads but the error type is important for some clients:
- EventSource may try to reconnect if error is not AccessControl
- XMLHttpRequest will send abort events in case of Cancellation errors and timeout events in case of Timeout errors
* loader/CrossOriginPreflightChecker.cpp:
(WebCore::CrossOriginPreflightChecker::notifyFinished): Setting error type to AccessControl except in case of Timeout.
(WebCore::CrossOriginPreflightChecker::doPreflight): Ditto.
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::preflightFailure): Removing ASSERT since Timeout errors may be returned.
* platform/network/ResourceErrorBase.h:
(WebCore::ResourceErrorBase::isGeneral): New getter.
LayoutTests:
* http/tests/xmlhttprequest/on-network-timeout-error-during-preflight-expected.txt: Added.
* http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html: Added.
* tests-options.json: Marking test as slow.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTeststestsoptionsjson">trunk/LayoutTests/tests-options.json</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreloaderCrossOriginPreflightCheckercpp">trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp</a></li>
<li><a href="#trunkSourceWebCoreloaderDocumentThreadableLoadercpp">trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformnetworkResourceErrorBaseh">trunk/Source/WebCore/platform/network/ResourceErrorBase.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestshttptestsxmlhttprequestonnetworktimeouterrorduringpreflightexpectedtxt">trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight-expected.txt</a></li>
<li><a href="#trunkLayoutTestshttptestsxmlhttprequestonnetworktimeouterrorduringpreflighthtml">trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (206523 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-09-28 16:34:06 UTC (rev 206523)
+++ trunk/LayoutTests/ChangeLog 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2016-09-28 Youenn Fablet <youenn@apple.com>
+
+ WebCore::ResourceErrorBase::setType is crashing
+ https://bugs.webkit.org/show_bug.cgi?id=162484
+ <rdar://problem/28390828>
+
+ Reviewed by Alex Christensen.
+
+ * http/tests/xmlhttprequest/on-network-timeout-error-during-preflight-expected.txt: Added.
+ * http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html: Added.
+ * tests-options.json: Marking test as slow.
+
</ins><span class="cx"> 2016-09-28 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><span class="cx"> [MSE][Mac] In SourceBufferPrivateAVFObjC::abort(), support reseting parser to the last appended initialization segment.
</span></span></pre></div>
<a id="trunkLayoutTestshttptestsxmlhttprequestonnetworktimeouterrorduringpreflightexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight-expected.txt (0 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight-expected.txt 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+
+PASS Testing timing out preflight
+
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestsxmlhttprequestonnetworktimeouterrorduringpreflighthtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html (0 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -0,0 +1,30 @@
</span><ins>+<!doctype html>
+<html>
+ <head>
+ <title>XMLHttpRequest: A timeout network error during preflight should end up in a timeout event</title>
+ <script src="/js-test-resources/testharness.js"></script>
+ <script src="/js-test-resources/testharnessreport.js"></script>
+ </head>
+ <body>
+ <div id="log"></div>
+ <script>
+ var test = async_test("Testing timing out preflight")
+ test.step(function() {
+ var client = new XMLHttpRequest()
+ var url = "http://localhost:8000/misc/resources/delayed-log.php?delay=10000000"
+ client.open("GET", url, true)
+ client.setRequestHeader("X-Custom", "test")
+ client.hasTimedout = false;
+ client.ontimeout = test.step_func(function () {
+ client.hasTimedout = true
+ })
+ client.onloadend = test.step_func(function () {
+ assert_true(client.hasTimedout, "xhr should have timed out")
+ test.done()
+ })
+ client.send(null)
+ })
+ </script>
+ </body>
+</html>
+
</ins></span></pre></div>
<a id="trunkLayoutTeststestsoptionsjson"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/tests-options.json (206523 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/tests-options.json 2016-09-28 16:34:06 UTC (rev 206523)
+++ trunk/LayoutTests/tests-options.json 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -1,4 +1,7 @@
</span><span class="cx"> {
</span><ins>+ "http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html": [
+ "slow"
+ ],
</ins><span class="cx"> "imported/w3c/syntax/parsing/html5lib_adoption01.html": [
</span><span class="cx"> "slow"
</span><span class="cx"> ],
</span><span class="lines">@@ -503,4 +506,4 @@
</span><span class="cx"> "imported/w3c/web-platform-tests/media-source/mediasource-redundant-seek.html": [
</span><span class="cx"> "slow"
</span><span class="cx"> ]
</span><del>-}
</del><span class="cx">\ No newline at end of file
</span><ins>+}
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (206523 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-09-28 16:34:06 UTC (rev 206523)
+++ trunk/Source/WebCore/ChangeLog 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2016-09-28 Youenn Fablet <youenn@apple.com>
+
+ WebCore::ResourceErrorBase::setType is crashing
+ https://bugs.webkit.org/show_bug.cgi?id=162484
+ <rdar://problem/28390828>
+
+ Reviewed by Alex Christensen.
+
+ Test: http/tests/xmlhttprequest/on-network-timeout-error-during-preflight.html
+
+ Behavior is slightly changed as we are no longer casting Timeout preflight errors as AccessControl errors.
+ This is more inline with fetch spec which prescribes to send back any error received by preflight as response error for fetch.
+
+ Ideally, we should not need to change errors received during preflight loads but the error type is important for some clients:
+ - EventSource may try to reconnect if error is not AccessControl
+ - XMLHttpRequest will send abort events in case of Cancellation errors and timeout events in case of Timeout errors
+
+ * loader/CrossOriginPreflightChecker.cpp:
+ (WebCore::CrossOriginPreflightChecker::notifyFinished): Setting error type to AccessControl except in case of Timeout.
+ (WebCore::CrossOriginPreflightChecker::doPreflight): Ditto.
+ * loader/DocumentThreadableLoader.cpp:
+ (WebCore::DocumentThreadableLoader::preflightFailure): Removing ASSERT since Timeout errors may be returned.
+ * platform/network/ResourceErrorBase.h:
+ (WebCore::ResourceErrorBase::isGeneral): New getter.
+
</ins><span class="cx"> 2016-09-28 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><span class="cx"> PiP shows incorrect state of play button.
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderCrossOriginPreflightCheckercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp (206523 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp 2016-09-28 16:34:06 UTC (rev 206523)
+++ trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -91,7 +91,11 @@
</span><span class="cx"> ASSERT_UNUSED(resource, resource == m_resource);
</span><span class="cx"> if (m_resource->loadFailedOrCanceled()) {
</span><span class="cx"> ResourceError preflightError = m_resource->resourceError();
</span><del>- preflightError.setType(ResourceError::Type::AccessControl);
</del><ins>+ // If the preflight was cancelled by underlying code, it probably means the request was blocked due to some access control policy.
+ // FIXME:: According fetch, we should just pass the error to the layer above. But this may impact some clients like XHR or EventSource.
+ if (preflightError.isNull() || preflightError.isCancellation() || preflightError.isGeneral())
+ preflightError.setType(ResourceError::Type::AccessControl);
+
</ins><span class="cx"> m_loader.preflightFailure(m_resource->identifier(), preflightError);
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="lines">@@ -133,9 +137,11 @@
</span><span class="cx">
</span><span class="cx"> unsigned identifier = loader.document().frame()->loader().loadResourceSynchronously(preflightRequest, DoNotAllowStoredCredentials, ClientCredentialPolicy::CannotAskClientForCredentials, error, response, data);
</span><span class="cx">
</span><del>- // FIXME: Investigate why checking for response httpStatusCode here. In particular, can we have a not-null error and a 2XX response.
- if (!error.isNull() && response.httpStatusCode() <= 0) {
- error.setType(ResourceError::Type::AccessControl);
</del><ins>+ if (!error.isNull()) {
+ // If the preflight was cancelled by underlying code, it probably means the request was blocked due to some access control policy.
+ // FIXME:: According fetch, we should just pass the error to the layer above. But this may impact some clients like XHR or EventSource.
+ if (error.isCancellation() || error.isGeneral())
+ error.setType(ResourceError::Type::AccessControl);
</ins><span class="cx"> loader.preflightFailure(identifier, error);
</span><span class="cx"> return;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderDocumentThreadableLoadercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp (206523 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2016-09-28 16:34:06 UTC (rev 206523)
+++ trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -341,7 +341,6 @@
</span><span class="cx">
</span><span class="cx"> void DocumentThreadableLoader::preflightFailure(unsigned long identifier, const ResourceError& error)
</span><span class="cx"> {
</span><del>- ASSERT(error.isAccessControl());
</del><span class="cx"> m_preflightChecker = Nullopt;
</span><span class="cx">
</span><span class="cx"> InspectorInstrumentation::didFailLoading(m_document.frame(), m_document.frame()->loader().documentLoader(), identifier, error);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformnetworkResourceErrorBaseh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/network/ResourceErrorBase.h (206523 => 206524)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/network/ResourceErrorBase.h 2016-09-28 16:34:06 UTC (rev 206523)
+++ trunk/Source/WebCore/platform/network/ResourceErrorBase.h 2016-09-28 16:37:26 UTC (rev 206524)
</span><span class="lines">@@ -53,6 +53,7 @@
</span><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> bool isNull() const { return m_type == Type::Null; }
</span><ins>+ bool isGeneral() const { return m_type == Type::General; }
</ins><span class="cx"> bool isAccessControl() const { return m_type == Type::AccessControl; }
</span><span class="cx"> bool isCancellation() const { return m_type == Type::Cancellation; }
</span><span class="cx"> bool isTimeout() const { return m_type == Type::Timeout; }
</span></span></pre>
</div>
</div>
</body>
</html>