ignore aborted response in http caching

components/net/fetch/methods.rs

@@ -29,6 +29,7 @@ use std::io::Read;
 use std::mem;
 use std::str;
 use std::sync::{Arc, Mutex};
+use std::sync::atomic::Ordering;
 use std::sync::mpsc::{Sender, Receiver};
 use subresource_integrity::is_response_integrity_valid;
 
@@ -422,7 +423,7 @@ fn wait_for_response(response: &mut Response, target: Target, done_chan: &mut Do
                 },
                 Data::Done => break,
                 Data::Cancelled => {
-                    response.aborted = true;
+                    response.aborted.store(true, Ordering::Relaxed);
                     break;
                 }
             }

components/net/http_cache.rs

@@ -22,6 +22,7 @@ use servo_url::ServoUrl;
 use std::collections::HashMap;
 use std::str;
 use std::sync::{Arc, Mutex};
+use std::sync::atomic::{AtomicBool, Ordering};
 use time;
 use time::{Duration, Tm};
 
@@ -63,7 +64,8 @@ struct CachedResource {
     raw_status: Option<(u16, Vec<u8>)>,
     url_list: Vec<ServoUrl>,
     expires: Duration,
-    last_validated: Tm
+    last_validated: Tm,
+    aborted: Arc<AtomicBool>,
 }
 
 /// Metadata about a loaded resource, such as is obtained from HTTP headers.
@@ -281,6 +283,7 @@ fn create_cached_response(request: &Request, cached_resource: &CachedResource, c
     response.https_state = cached_resource.https_state.clone();
     response.referrer = request.referrer.to_url().cloned();
     response.referrer_policy = request.referrer_policy.clone();
+    response.aborted = cached_resource.aborted.clone();
     let expires = cached_resource.expires;
     let adjusted_expires = get_expiry_adjustment_from_request_headers(request, expires);
     let now = Duration::seconds(time::now().to_timespec().sec);
@@ -308,7 +311,8 @@ fn create_resource_with_bytes_from_resource(bytes: &[u8], resource: &CachedResou
         raw_status: Some((206, b"Partial Content".to_vec())),
         url_list: resource.url_list.clone(),
         expires: resource.expires.clone(),
-        last_validated: resource.last_validated.clone()
+        last_validated: resource.last_validated.clone(),
+        aborted: Arc::new(AtomicBool::new(false)),
     }
 }
 
@@ -479,9 +483,17 @@ impl HttpCache {
             return None;
         }
         let entry_key = CacheKey::new(request.clone());
-        let resources = self.entries.get(&entry_key)?.clone();
+        let resources = self.entries.get(&entry_key)?.into_iter().filter(|r| {
+            match *r.body.lock().unwrap() {
+                ResponseBody::Done(_) => !r.aborted.load(Ordering::Relaxed),
+                // TODO: use fetch::methods::DoneChannel, in order to be able to
+                // construct a response with a body in ResponseBody::Receiving mode.
+                ResponseBody::Receiving(_) => false,
+                ResponseBody::Empty => true
+            }
+        });
         let mut candidates = vec![];
-        for cached_resource in resources.iter() {
+        for cached_resource in resources {
             let mut can_be_constructed = true;
             let cached_headers = cached_resource.metadata.headers.lock().unwrap();
             let original_request_headers = cached_resource.request_headers.lock().unwrap();
@@ -642,7 +654,8 @@ impl HttpCache {
             raw_status: response.raw_status.clone(),
             url_list: response.url_list.clone(),
             expires: expiry,
-            last_validated: time::now()
+            last_validated: time::now(),
+            aborted: response.aborted.clone()
         };
         let entry = self.entries.entry(entry_key).or_insert(vec![]);
         entry.push(entry_resource);

components/net_traits/response.rs

@@ -10,6 +10,7 @@ use hyper::status::StatusCode;
 use hyper_serde::Serde;
 use servo_url::ServoUrl;
 use std::sync::{Arc, Mutex};
+use std::sync::atomic::AtomicBool;
 
 /// [Response type](https://fetch.spec.whatwg.org/#concept-response-type)
 #[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
@@ -113,7 +114,8 @@ pub struct Response {
     /// whether or not to try to return the internal_response when asked for actual_response
     pub return_internal: bool,
     /// https://fetch.spec.whatwg.org/#concept-response-aborted
-    pub aborted: bool,
+    #[ignore_malloc_size_of = "AtomicBool heap size undefined"]
+    pub aborted: Arc<AtomicBool>,
 }
 
 impl Response {
@@ -135,7 +137,7 @@ impl Response {
             location_url: None,
             internal_response: None,
             return_internal: true,
-            aborted: false,
+            aborted: Arc::new(AtomicBool::new(false)),
         }
     }
 
@@ -165,7 +167,7 @@ impl Response {
             location_url: None,
             internal_response: None,
             return_internal: true,
-            aborted: false,
+            aborted: Arc::new(AtomicBool::new(false)),
         }
     }
 

tests/wpt/mozilla/meta/MANIFEST.json

@@ -12478,6 +12478,11 @@
      {}
     ]
    ],
+   "mozilla/resources/http-cache-trickle.py": [
+    [
+     {}
+    ]
+   ],
    "mozilla/resources/http-cache.js": [
     [
      {}
@@ -33245,6 +33250,12 @@
      {}
     ]
    ],
+   "mozilla/http-cache-xhr.html": [
+    [
+     "/_mozilla/mozilla/http-cache-xhr.html",
+     {}
+    ]
+   ],
    "mozilla/http-cache.html": [
     [
      "/_mozilla/mozilla/http-cache.html",
@@ -66348,6 +66359,10 @@
    "592f69ee432ba5bc7a2f2649e72e083d21393496",
    "testharness"
   ],
+  "mozilla/http-cache-xhr.html": [
+   "0f8e347991f25a08ba5f8bdff0ef99055e33e7a1",
+   "testharness"
+  ],
   "mozilla/http-cache.html": [
    "33827dc9bdb0efcbcae4f730086693be315cfc14",
    "testharness"
@@ -72024,8 +72039,12 @@
    "78686147f85e4146e7fc58c1f67a613f65b099a2",
    "support"
   ],
+  "mozilla/resources/http-cache-trickle.py": [
+   "48f4e32ec2e1c1b6d47e89254045b398c1d84d40",
+   "support"
+  ],
   "mozilla/resources/http-cache.js": [
-   "c6b1ee9def26d4e12a1b93e551c225f82b4717c2",
+   "4bf71e1f328e778990eb756741a3be58f4f57ef6",
    "support"
   ],
   "mozilla/resources/iframe_contentDocument_inner.html": [

tests/wpt/mozilla/meta/mozilla/http-cache-xhr.html.ini

@@ -0,0 +1,2 @@
+[http-cache-xhr.html]
+  type: testharness

tests/wpt/mozilla/tests/mozilla/http-cache-xhr.html

@@ -0,0 +1,73 @@
+<html>
+<head>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/utils.js"></script>
+<script src="resources/http-cache.js"></script>
+<script>
+  var test = async_test('The response from an aborted XHR request should not be cached');
+  test.step(function() {
+    var client = new XMLHttpRequest();
+    var requests = [
+        {
+          response_status: [200, 'OK'],
+          "response_headers": [
+            ['Expires', http_date(100000)],
+            ['Last-Modified', http_date(0)]
+          ],
+          response_body: '12'
+        },
+        {
+          response_status: [200, 'OK'],
+          "response_headers": [
+            ['Expires', http_date(100000)],
+            ['Last-Modified', http_date(0)]
+          ],
+          response_body: '12'
+        },
+        {
+          response_status: [200, 'OK'],
+          "response_headers": [
+            ['Expires', http_date(100000)],
+            ['Last-Modified', http_date(0)]
+          ],
+          response_body: '12'
+        }
+    ];
+    var uuid = token();
+    var url = make_url(uuid, requests, 0);
+    url += '&trickle=true';
+    client.onprogress = test.step_func(function() {
+      var client2 = new XMLHttpRequest();
+      client2.onloadend = test.step_func(function(event) {
+        server_state(uuid).then(test.step_func(function(state){
+          // Two server hits for two requests, no caching.
+          assert_equals(state.length, 2);
+          // The empty, aborted response.
+          assert_equals(client.response, "");
+          assert_equals(client2.response, "12");
+        }));
+        var client3 = new XMLHttpRequest();
+        client3.onloadend = test.step_func(function(event) {
+          server_state(uuid).then(test.step_func(function(state) {
+            // No server hits, the response from client2 was cached and re-used.
+            assert_equals(state.length, 0)
+            assert_equals(client3.response, '12');
+            test.done();
+          }));
+        });
+        client3.open("GET", url);
+        client3.send();
+      });
+      client.onabort = test.step_func(function() {
+        client2.open("GET", url);
+        client2.send();
+      });
+      client.abort();
+    });
+    client.open("GET", url);
+    client.send();
+  });
+</script>
+</head>
+</html>

tests/wpt/mozilla/tests/mozilla/resources/http-cache-trickle.py

@@ -0,0 +1,64 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+
+import time
+from json import JSONEncoder, JSONDecoder
+from base64 import b64decode
+
+
+def main(request, response):
+    uuid = request.GET.first("token", None)
+    if "querystate" in request.GET:
+        response.headers.set("Content-Type", "text/plain")
+        return JSONEncoder().encode(request.server.stash.take(uuid))
+
+    server_state = request.server.stash.take(uuid)
+    if not server_state:
+        server_state = []
+
+    requests = JSONDecoder().decode(b64decode(request.GET.first("info", "")))
+    config = requests[len(server_state)]
+
+    state = dict()
+    state["request_method"] = request.method
+    state["request_headers"] = dict([[h.lower(), request.headers[h]] for h in request.headers])
+    server_state.append(state)
+    request.server.stash.put(uuid, server_state)
+
+    note_headers = ['content-type', 'access-control-allow-origin', 'last-modified', 'etag']
+    noted_headers = {}
+    for header in config.get('response_headers', []):
+        if header[0].lower() in ["location", "content-location"]:  # magic!
+            header[1] = "%s&target=%s" % (request.url, header[1])
+        response.headers.set(header[0], header[1])
+        if header[0].lower() in note_headers:
+            noted_headers[header[0].lower()] = header[1]
+
+    if "access-control-allow-origin" not in noted_headers:
+        response.headers.set("Access-Control-Allow-Origin", "*")
+    if "content-type" not in noted_headers:
+        response.headers.set("Content-Type", "text/plain")
+    response.headers.set("Server-Request-Count", len(server_state))
+
+    code, phrase = config.get("response_status", [200, "OK"])
+
+    if request.headers.get("If-Modified-Since", False) == noted_headers.get('last-modified', True):
+        code, phrase = [304, "Not Modified"]
+    if request.headers.get("If-None-Match", False) == noted_headers.get('etag', True):
+        code, phrase = [304, "Not Modified"]
+
+    response.status = (code, phrase)
+
+    if request.GET.first("trickle", None):
+        response.write_status_headers()
+        for byte in config.get("response_body", uuid):
+            response.writer.write_content(byte)
+            time.sleep(1)
+
+    content = config.get("response_body", uuid)
+    if code in [204, 304]:
+        return ""
+    else:
+        return content

tests/wpt/mozilla/tests/mozilla/resources/http-cache.js

@@ -40,11 +40,11 @@ function make_url(uuid, requests, idx) {
   if ("query_arg" in requests[idx]) {
     arg = "&target=" + requests[idx].query_arg;
   }
-  return "/fetch/http-cache/resources/http-cache.py?token=" + uuid + "&info=" + btoa(JSON.stringify(requests)) + arg;
+  return "resources/http-cache-trickle.py?token=" + uuid + "&info=" + btoa(JSON.stringify(requests)) + arg;
 }
 
 function server_state(uuid) {
-  return fetch("/fetch/http-cache/resources/http-cache.py?querystate&token=" + uuid)
+  return fetch("resources/http-cache-trickle.py?querystate&token=" + uuid)
     .then(function(response) {
       return response.text();
     }).then(function(text) {