Bug 1280590 - nsICookieManager2.cookieExists must use the originAttributes, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Fri, 30 Sep 2016 08:24:21 +0200
changeset 316032 9cba6697ae68563dfc87b4131dcf83683d9de8e7
parent 316031 3fc3826c761ee16c2c8c7bde129927f8e1466615
child 316033 e0178650c6742f4eb1340a1d0c8e8c5871e50d3d
push id30759
push userphilringnalda@gmail.com
push dateSat, 01 Oct 2016 06:25:09 +0000
treeherdermozilla-central@fcc62bbf09ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1280590
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1280590 - nsICookieManager2.cookieExists must use the originAttributes, r=smaug
CLOBBER
browser/components/sessionstore/SessionCookies.jsm
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsICookieManager2.idl
netwerk/test/TestCookie.cpp
--- a/CLOBBER
+++ b/CLOBBER
@@ -17,9 +17,9 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Merge day clobber
\ No newline at end of file
+Bug 1280590 - xpcshell failure in windows7
--- a/browser/components/sessionstore/SessionCookies.jsm
+++ b/browser/components/sessionstore/SessionCookies.jsm
@@ -110,24 +110,25 @@ var SessionCookiesInternal = {
 
     return hosts;
   },
 
   /**
    * Restores a given list of session cookies.
    */
   restore(cookies) {
+
     for (let cookie of cookies) {
       let expiry = "expiry" in cookie ? cookie.expiry : MAX_EXPIRY;
       let cookieObj = {
         host: cookie.host,
         path: cookie.path || "",
         name: cookie.name || ""
       };
-      if (!Services.cookies.cookieExists(cookieObj)) {
+      if (!Services.cookies.cookieExists(cookieObj, cookie.originAttributes || {})) {
         Services.cookies.add(cookie.host, cookie.path || "", cookie.name || "",
                              cookie.value, !!cookie.secure, !!cookie.httponly,
                              /* isSession = */ true, expiry, cookie.originAttributes || {});
       }
     }
   },
 
   /**
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -4351,20 +4351,52 @@ nsCookieService::PurgeCookies(int64_t aC
      aCurrentTimeInUsec - mDBState->cookieOldestTime));
 
   return removedList.forget();
 }
 
 // find whether a given cookie has been previously set. this is provided by the
 // nsICookieManager2 interface.
 NS_IMETHODIMP
-nsCookieService::CookieExists(nsICookie2 *aCookie,
-                              bool       *aFoundCookie)
+nsCookieService::CookieExists(nsICookie2* aCookie,
+                              JS::HandleValue aOriginAttributes,
+                              JSContext* aCx,
+                              uint8_t aArgc,
+                              bool* aFoundCookie)
 {
   NS_ENSURE_ARG_POINTER(aCookie);
+  NS_ENSURE_ARG_POINTER(aCx);
+  NS_ENSURE_ARG_POINTER(aFoundCookie);
+
+  MOZ_ASSERT(aArgc == 0 || aArgc == 1);
+
+  nsresult rv;
+  NeckoOriginAttributes attrs;
+
+  if (aArgc == 1) {
+    rv = InitializeOriginAttributes(&attrs,
+                                    aOriginAttributes,
+                                    aCx,
+                                    aArgc,
+                                    u"nsICookieManager2.cookieExists()",
+                                    u"2");
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  return CookieExistsNative(aCookie, &attrs, aFoundCookie);
+}
+
+NS_IMETHODIMP_(nsresult)
+nsCookieService::CookieExistsNative(nsICookie2* aCookie,
+                                    NeckoOriginAttributes* aOriginAttributes,
+                                    bool* aFoundCookie)
+{
+  NS_ENSURE_ARG_POINTER(aCookie);
+  NS_ENSURE_ARG_POINTER(aOriginAttributes);
+  NS_ENSURE_ARG_POINTER(aFoundCookie);
 
   if (!mDBState) {
     NS_WARNING("No DBState! Profile already closed?");
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsAutoCString host, name, path;
   nsresult rv = aCookie->GetHost(host);
@@ -4374,17 +4406,18 @@ nsCookieService::CookieExists(nsICookie2
   rv = aCookie->GetPath(path);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString baseDomain;
   rv = GetBaseDomainFromHost(host, baseDomain);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsListIter iter;
-  *aFoundCookie = FindCookie(DEFAULT_APP_KEY(baseDomain), host, name, path, iter);
+  *aFoundCookie = FindCookie(nsCookieKey(baseDomain, *aOriginAttributes),
+                             host, name, path, iter);
   return NS_OK;
 }
 
 // For a given base domain, find either an expired cookie or the oldest cookie
 // by lastAccessed time.
 void
 nsCookieService::FindStaleCookie(nsCookieEntry *aEntry,
                                  int64_t aCurrentTime,
--- a/netwerk/cookie/nsICookieManager2.idl
+++ b/netwerk/cookie/nsICookieManager2.idl
@@ -71,21 +71,33 @@ interface nsICookieManager2 : nsICookieM
                      in int64_t     aExpiry,
                      in NeckoOriginAttributesPtr aOriginAttributes);
 
   /**
    * Find whether a given cookie already exists.
    *
    * @param aCookie
    *        the cookie to look for
+   * @param aOriginAttributes
+   *        nsICookie2 contains an originAttributes but if nsICookie2 is
+   *        implemented in JS, we can't retrieve its originAttributes because
+   *        the getter is marked [implicit_jscontext]. This optional parameter
+   *        is a workaround.
    *
    * @return true if a cookie was found which matches the host, path, and name
    *         fields of aCookie
    */
-  boolean cookieExists(in nsICookie2 aCookie);
+  [implicit_jscontext, optional_argc]
+  boolean cookieExists(in nsICookie2 aCookie,
+                       [optional] in jsval aOriginAttributes);
+
+  [notxpcom]
+  nsresult cookieExistsNative(in nsICookie2 aCookie,
+                              in NeckoOriginAttributesPtr aOriginAttributes,
+                              out boolean aExists);
 
   /**
    * Count how many cookies exist within the base domain of 'aHost'.
    * Thus, for a host "weather.yahoo.com", the base domain would be "yahoo.com",
    * and any host or domain cookies for "yahoo.com" and its subdomains would be
    * counted.
    *
    * @param aHost
--- a/netwerk/test/TestCookie.cpp
+++ b/netwerk/test/TestCookie.cpp
@@ -752,45 +752,45 @@ main(int32_t argc, char *argv[])
       GetACookie(cookieService, "http://cookiemgr.test/foo/", nullptr, getter_Copies(cookie));
       rv[6] = CheckResult(cookie.get(), MUST_CONTAIN, "test2=yes");
       GetACookieNoHttp(cookieService, "http://cookiemgr.test/foo/", getter_Copies(cookie));
       rv[7] = CheckResult(cookie.get(), MUST_NOT_CONTAIN, "test2=yes");
       // check CountCookiesFromHost()
       uint32_t hostCookies = 0;
       rv[8] = NS_SUCCEEDED(cookieMgr2->CountCookiesFromHost(NS_LITERAL_CSTRING("cookiemgr.test"), &hostCookies)) &&
               hostCookies == 2;
-      // check CookieExists() using the third cookie
+      // check CookieExistsNative() using the third cookie
       bool found;
-      rv[9] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && found;
+      rv[9] = NS_SUCCEEDED(cookieMgr2->CookieExistsNative(newDomainCookie, &attrs,  &found)) && found;
 
 
       // remove the cookie, block it, and ensure it can't be added again
       rv[10] = NS_SUCCEEDED(cookieMgr->RemoveNative(NS_LITERAL_CSTRING("new.domain"), // domain
                                                     NS_LITERAL_CSTRING("test3"),      // name
                                                     NS_LITERAL_CSTRING("/rabbit"),    // path
                                                     true,                             // is blocked
                                                     &attrs));                         // originAttributes
-      rv[11] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && !found;
+      rv[11] = NS_SUCCEEDED(cookieMgr2->CookieExistsNative(newDomainCookie, &attrs, &found)) && !found;
       rv[12] = NS_SUCCEEDED(cookieMgr2->AddNative(NS_LITERAL_CSTRING("new.domain"),     // domain
                                             NS_LITERAL_CSTRING("/rabbit"),        // path
                                             NS_LITERAL_CSTRING("test3"),          // name
                                             NS_LITERAL_CSTRING("yes"),            // value
                                             false,                             // is secure
                                             false,                             // is httponly
                                             true,                              // is session
                                             INT64_MIN,                            // expiry time
                                             &attrs));                         // originAttributes
-      rv[13] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && !found;
+      rv[13] = NS_SUCCEEDED(cookieMgr2->CookieExistsNative(newDomainCookie, &attrs, &found)) && !found;
       // sleep four seconds, to make sure the second cookie has expired
       PR_Sleep(4 * PR_TicksPerSecond());
-      // check that both CountCookiesFromHost() and CookieExists() count the
+      // check that both CountCookiesFromHost() and CookieExistsNative() count the
       // expired cookie
       rv[14] = NS_SUCCEEDED(cookieMgr2->CountCookiesFromHost(NS_LITERAL_CSTRING("cookiemgr.test"), &hostCookies)) &&
               hostCookies == 2;
-      rv[15] = NS_SUCCEEDED(cookieMgr2->CookieExists(expiredCookie, &found)) && found;
+      rv[15] = NS_SUCCEEDED(cookieMgr2->CookieExistsNative(expiredCookie, &attrs, &found)) && found;
       // double-check RemoveAll() using the enumerator
       rv[16] = NS_SUCCEEDED(cookieMgr->RemoveAll());
       rv[17] = NS_SUCCEEDED(cookieMgr->GetEnumerator(getter_AddRefs(enumerator))) &&
                NS_SUCCEEDED(enumerator->HasMoreElements(&more)) &&
                !more;
 
       allTestsPassed = PrintResult(rv, 18) && allTestsPassed;