Bug 1280590 - nsICookieManager2.cookieExists must use the originAttributes, r=smaug
☠☠ backed out by 98eb59193369 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 29 Sep 2016 12:02:30 +0200
changeset 315820 826cc48624a31a2755c23bfe83535311df1567bc
parent 315819 6a996a75330fb7d4376b4ed6d439126d8b5a50a2
child 315821 01cd7a8a158d425c216cb319898832c4891ec35e
push id20634
push usercbook@mozilla.com
push dateFri, 30 Sep 2016 10:10:13 +0000
treeherderfx-team@afe79b010d13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1280590
milestone52.0a1
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,61 @@ 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 == 0) {
+    JS::Rooted<JS::Value> value(aCx);
+    rv = aCookie->GetOriginAttributes(aCx, &value);
+    // We ignore this failure because nsICookie2 could be implemented in JS and
+    // this getter will failed because marked [implicit_jscontext].
+    if (NS_SUCCEEDED(rv) &&
+        (!value.isObject() || !attrs.Init(aCx, value))) {
+      return NS_ERROR_INVALID_ARG;
+    }
+  } else {
+    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 +4415,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;