Address review comments.

2 files changed, 35 insertions, 14 deletions

diff --git a/components/net/cookie.rs b/components/net/cookie.rs
index 73a53efbd4f..eb3c1a61a97 100644
--- a/components/net/cookie.rs
+++ b/components/net/cookie.rs
@@ -199,8 +199,7 @@ fn fn_cookie_constructor() {
 
     // cookie domains test
     let cookie = cookie_rs::Cookie::parse(" baz = bar; Domain =  ").unwrap();
-    assert!(Cookie::new_wrapped(cookie, url, CookieSource::HTTP).is_some());
-    let cookie = cookie_rs::Cookie::parse(" baz = bar; Domain =  ").unwrap();
+    assert!(Cookie::new_wrapped(cookie.clone(), url, CookieSource::HTTP).is_some());
     let cookie = Cookie::new_wrapped(cookie, url, CookieSource::HTTP).unwrap();
     assert!(cookie.cookie.domain.as_ref().unwrap().as_slice() == "example.com");
 
diff --git a/components/net/cookie_storage.rs b/components/net/cookie_storage.rs
index dc69c146a5b..deeb9c534e2 100644
--- a/components/net/cookie_storage.rs
+++ b/components/net/cookie_storage.rs
@@ -76,6 +76,21 @@ impl CookieStorage {
         self.cookies.push(cookie);
     }
 
+    fn cookie_comparator(a: &Cookie, b: &Cookie) -> Ordering {
+        let a_path_len = a.cookie.path.as_ref().map(|p| p.len()).unwrap_or(0);
+        let b_path_len = b.cookie.path.as_ref().map(|p| p.len()).unwrap_or(0);
+        match a_path_len.cmp(&b_path_len) {
+            Ordering::Equal => {
+                let a_creation_time = a.creation_time.to_timespec();
+                let b_creation_time = b.creation_time.to_timespec();
+                a_creation_time.cmp(&b_creation_time)
+            }
+            // Ensure that longer paths are sorted earlier than shorter paths
+            Ordering::Greater => Ordering::Less,
+            Ordering::Less => Ordering::Greater,
+        }
+    }
+
     // http://tools.ietf.org/html/rfc6265#section-5.4
     pub fn cookies_for_url(&mut self, url: &Url, source: CookieSource) -> Option<String> {
         let filterer = |&:c: &&mut Cookie| -> bool {
@@ -87,18 +102,7 @@ impl CookieStorage {
 
         // Step 2
         let mut url_cookies: Vec<&mut Cookie> = self.cookies.iter_mut().filter(filterer).collect();
-        url_cookies.sort_by(|a, b| {
-            let a_path_len = a.cookie.path.as_ref().map(|p| p.len()).unwrap_or(0);
-            let b_path_len = b.cookie.path.as_ref().map(|p| p.len()).unwrap_or(0);
-            match a_path_len.cmp(&b_path_len) {
-                Ordering::Equal => {
-                    let a_creation_time = a.creation_time.to_timespec();
-                    let b_creation_time = b.creation_time.to_timespec();
-                    a_creation_time.cmp(&b_creation_time)
-                }
-                result => result
-            }
-        });
+        url_cookies.sort_by(|a, b| CookieStorage::cookie_comparator(*a, *b));
 
         let reducer = |&:acc: String, c: &mut &mut Cookie| -> String {
             // Step 3
@@ -119,3 +123,21 @@ impl CookieStorage {
         }
     }
 }
+
+#[test]
+fn test_sort_order() {
+    use cookie_rs;
+    let url = &Url::parse("http://example.com/foo").unwrap();
+    let a_wrapped = cookie_rs::Cookie::parse("baz=bar; Path=/foo/bar/").unwrap();
+    let a = Cookie::new_wrapped(a_wrapped.clone(), url, CookieSource::HTTP).unwrap();
+    let a_prime = Cookie::new_wrapped(a_wrapped, url, CookieSource::HTTP).unwrap();
+    let b = cookie_rs::Cookie::parse("baz=bar;Path=/foo/bar/baz/").unwrap();
+    let b = Cookie::new_wrapped(b, url, CookieSource::HTTP).unwrap();
+
+    assert!(b.cookie.path.as_ref().unwrap().len() > a.cookie.path.as_ref().unwrap().len());
+    assert!(CookieStorage::cookie_comparator(&a, &b) == Ordering::Greater);
+    assert!(CookieStorage::cookie_comparator(&b, &a) == Ordering::Less);
+    assert!(CookieStorage::cookie_comparator(&a, &a_prime) == Ordering::Less);
+    assert!(CookieStorage::cookie_comparator(&a_prime, &a) == Ordering::Greater);
+    assert!(CookieStorage::cookie_comparator(&a, &a) == Ordering::Equal);
+}