Bug 1259169 - nsICookieManager::remove() should be back-compatible, r=jdm

browser/base/content/sanitize.js

@@ -249,7 +249,7 @@ Sanitizer.prototype = {
               if (cookie.creationTime > range[0]) {
                 // This cookie was created after our cutoff, clear it
                 cookieMgr.remove(cookie.host, cookie.name, cookie.path,
-                                 cookie.originAttributes, false);
+                                 false, cookie.originAttributes);
 
                 if (++yieldCounter % YIELD_PERIOD == 0) {
                   yield new Promise(resolve => setTimeout(resolve, 0)); // Don't block the main thread too long

browser/components/migration/MSMigrationUtils.jsm

@@ -623,7 +623,7 @@ Cookies.prototype = {
       // a domain cookie.  See bug 222343.
       if (host.length > 0) {
         // Fist delete any possible extant matching host cookie.
-        Services.cookies.remove(host, name, path, {}, false);
+        Services.cookies.remove(host, name, path, false, {});
         // Now make it a domain cookie.
         if (host[0] != "." && !hostIsIPAddress(host))
           host = "." + host;

browser/components/preferences/cookies.js

@@ -591,7 +591,7 @@ var gCookiesWindow = {
     for (var i = 0; i < deleteItems.length; ++i) {
       var item = deleteItems[i];
       this._cm.remove(item.host, item.name, item.path,
-                      item.originAttributes, blockFutureCookies);
+                      blockFutureCookies, item.originAttributes);
     }
   },
 

devtools/server/actors/storage.js

@@ -851,7 +851,7 @@ var cookieHelpers = {
       case "path":
         // Remove the edited cookie.
         Services.cookies.remove(origHost, origName, origPath,
-                                cookie.originAttributes, false);
+                                false, cookie.originAttributes);
         break;
     }
 
@@ -894,8 +894,8 @@ var cookieHelpers = {
           cookie.host,
           cookie.name,
           cookie.path,
-          cookie.originAttributes,
+          false,
-          false
+          cookie.originAttributes
         );
       }
     }

devtools/shared/gcli/commands/cookie.js

@@ -133,7 +133,7 @@ exports.items = [
         if (isCookieAtHost(cookie, host)) {
           if (cookie.name == args.name) {
             cookieMgr.remove(cookie.host, cookie.name, cookie.path,
-                             cookie.originAttributes, false);
+                             false, cookie.originAttributes);
           }
         }
       }

extensions/cookie/test/unit/test_bug526789.js

@@ -28,9 +28,9 @@ function run_test() {
   do_check_throws(function() {
     cm.countCookiesFromHost("..baz.com");
   }, Cr.NS_ERROR_ILLEGAL_VALUE);
-  cm.remove("BAZ.com.", "foo", "/", {}, false);
+  cm.remove("BAZ.com.", "foo", "/", false, {});
   do_check_eq(cm.countCookiesFromHost("baz.com"), 1);
-  cm.remove("baz.com", "foo", "/", {}, false);
+  cm.remove("baz.com", "foo", "/", false, {});
   do_check_eq(cm.countCookiesFromHost("baz.com"), 0);
 
   // Test that 'baz.com' and 'baz.com.' are treated differently
@@ -40,9 +40,9 @@ function run_test() {
   do_check_eq(cm.countCookiesFromHost(".baz.com"), 0);
   do_check_eq(cm.countCookiesFromHost("baz.com."), 1);
   do_check_eq(cm.countCookiesFromHost(".baz.com."), 1);
-  cm.remove("baz.com", "foo", "/", {}, false);
+  cm.remove("baz.com", "foo", "/", false, {});
   do_check_eq(cm.countCookiesFromHost("baz.com."), 1);
-  cm.remove("baz.com.", "foo", "/", {}, false);
+  cm.remove("baz.com.", "foo", "/", false, {});
   do_check_eq(cm.countCookiesFromHost("baz.com."), 0);
 
   // test that domain cookies are illegal for IP addresses, aliases such as
@@ -174,10 +174,10 @@ function run_test() {
   }, Cr.NS_ERROR_ILLEGAL_VALUE);
   do_check_eq(getCookieCount(), 1);
 
-  cm.remove("", "foo2", "/", {}, false);
+  cm.remove("", "foo2", "/", false, {});
   do_check_eq(getCookieCount(), 0);
   do_check_throws(function() {
-    cm.remove(".", "foo3", "/", {}, false);
+    cm.remove(".", "foo3", "/", false, {});
   }, Cr.NS_ERROR_ILLEGAL_VALUE);
 
   // test that the 'domain' attribute accepts a leading dot for IP addresses,

extensions/cookie/test/unit/test_cookies_profile_close.js

@@ -58,7 +58,7 @@ function do_run_test() {
   }, Cr.NS_ERROR_NOT_AVAILABLE);
 
   do_check_throws(function() {
-    Services.cookiemgr.remove("foo.com", "", "oh4", {}, false);
+    Services.cookiemgr.remove("foo.com", "", "oh4", false, {});
   }, Cr.NS_ERROR_NOT_AVAILABLE);
 
   do_check_throws(function() {

extensions/cookie/test/unit/test_cookies_read.js

@@ -86,11 +86,11 @@ function do_run_test() {
   // remove some of the cookies, in both reverse and forward order
   for (let i = 100; i-- > 0; ) {
     let host = i.toString() + ".com";
-    Services.cookiemgr.remove(host, "oh", "/", {}, false);
+    Services.cookiemgr.remove(host, "oh", "/", false, {});
   }
   for (let i = CMAX - 100; i < CMAX; ++i) {
     let host = i.toString() + ".com";
-    Services.cookiemgr.remove(host, "oh", "/", {}, false);
+    Services.cookiemgr.remove(host, "oh", "/", false, {});
   }
 
   // check the count

netwerk/cookie/nsCookieService.cpp

@@ -12,11 +12,13 @@
 #include "mozilla/net/NeckoCommon.h"
 
 #include "nsCookieService.h"
+#include "nsContentUtils.h"
 #include "nsIServiceManager.h"
 
 #include "nsIIOService.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
+#include "nsIScriptError.h"
 #include "nsICookiePermission.h"
 #include "nsIURI.h"
 #include "nsIURL.h"
@@ -2366,21 +2368,35 @@ NS_IMETHODIMP
 nsCookieService::Remove(const nsACString &aHost,
                         const nsACString &aName,
                         const nsACString &aPath,
-                        JS::HandleValue  aOriginAttributes,
                         bool             aBlocked,
-                        JSContext*       aCx)
+                        JS::HandleValue  aOriginAttributes,
+                        JSContext*       aCx,
+                        uint8_t          aArgc)
 {
+  MOZ_ASSERT(aArgc == 0 || aArgc == 1);
+  if (aArgc == 0) {
+    // This is supposed to be temporary and in 1 or 2 releases we want to
+    // have originAttributes param as mandatory. But for now, we don't want to
+    // break existing addons, so we write a console message to inform the addon
+    // developers about it.
+    nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
+                                    NS_LITERAL_CSTRING("Cookie Manager"),
+                                    nullptr,
+                                    nsContentUtils::eNECKO_PROPERTIES,
+                                    "nsICookieManagerRemoveDeprecated");
+  }
+
   NeckoOriginAttributes attrs;
   MOZ_ASSERT(attrs.Init(aCx, aOriginAttributes));
-  return RemoveNative(aHost, aName, aPath, &attrs, aBlocked);
+  return RemoveNative(aHost, aName, aPath, aBlocked, &attrs);
 }
 
 NS_IMETHODIMP_(nsresult)
 nsCookieService::RemoveNative(const nsACString &aHost,
                               const nsACString &aName,
                               const nsACString &aPath,
-                              NeckoOriginAttributes* aOriginAttributes,
+                              bool aBlocked,
-                              bool aBlocked)
+                              NeckoOriginAttributes* aOriginAttributes)
 {
   if (NS_WARN_IF(!aOriginAttributes)) {
     return NS_ERROR_FAILURE;

netwerk/cookie/nsICookieManager.idl

@@ -47,21 +47,24 @@ interface nsICookieManager : nsISupports
    *              dot must be present.
    * @param aName The name specified in the cookie
    * @param aPath The path for which the cookie was set
-   * @param aOriginAttributes The originAttributes of this cookie
+   * @param aOriginAttributes The originAttributes of this cookie. This
-   * @param aBlocked Indicates if cookies from this host should be permanently blocked
+   *                          attribute is optional to avoid breaking add-ons.
+   *                          In 1 or 2 releases it will be mandatory: bug 1260399.
+   * @param aBlocked Indicates if cookies from this host should be permanently
+   *                 blocked.
    *
    */
-  [implicit_jscontext]
+  [implicit_jscontext, optional_argc]
   void remove(in AUTF8String   aHost,
               in ACString      aName,
               in AUTF8String   aPath,
-              in jsval         aOriginAttributes,
+              in boolean       aBlocked,
-              in boolean       aBlocked);
+              [optional] in jsval aOriginAttributes);
 
   [notxpcom]
   nsresult removeNative(in AUTF8String   aHost,
                         in ACString      aName,
                         in AUTF8String   aPath,
-                        in NeckoOriginAttributesPtr aOriginAttributes,
+                        in boolean       aBlocked,
-                        in boolean       aBlocked);
+                        in NeckoOriginAttributesPtr aOriginAttributes);
 };

netwerk/locales/en-US/necko.properties

@@ -43,3 +43,6 @@ TrackingUriBlocked=The resource at "%1$S" was blocked because tracking protectio
 # LOCALIZATION NOTE (APIDeprecationWarning):
 # %1$S is the deprected API; %2$S is the API function that should be used.
 APIDeprecationWarning=Warning: '%1$S' deprecated, please use '%2$S'
+
+# LOCALIZATION NOTE (nsICookieManagerRemoveDeprecated): don't localize nsICookieManager.remove() and originAttributes.
+nsICookieManagerRemoveDeprecated="'nsICookieManager.remove()' is changed. Update your code and pass the correct originAttributes. Read more on MDN: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsICookieManager'

netwerk/test/TestCookie.cpp

@@ -665,8 +665,8 @@ main(int32_t argc, char *argv[])
       rv[10] = NS_SUCCEEDED(cookieMgr->RemoveNative(NS_LITERAL_CSTRING("new.domain"), // domain
                                                     NS_LITERAL_CSTRING("test3"),      // name
                                                     NS_LITERAL_CSTRING("/rabbit"),    // path
-                                                    &attrs,                           // originAttributes
+                                                    true,                             // is blocked
-                                                    true));                           // is blocked
+                                                    &attrs));                         // originAttributes
       rv[11] = NS_SUCCEEDED(cookieMgr2->CookieExists(newDomainCookie, &found)) && !found;
       rv[12] = NS_SUCCEEDED(cookieMgr2->Add(NS_LITERAL_CSTRING("new.domain"),     // domain
                                             NS_LITERAL_CSTRING("/rabbit"),        // path

testing/marionette/driver.js

@@ -2285,8 +2285,8 @@ GeckoDriver.prototype.deleteAllCookies = function*(cmd, resp) {
         cookie.host,
         cookie.name,
         cookie.path,
-        cookie.originAttributes,
+        false,
-        false);
+        cookie.originAttributes);
     return true;
   };
   this.mm.addMessageListener("Marionette:deleteCookie", cb);
@@ -2303,8 +2303,8 @@ GeckoDriver.prototype.deleteCookie = function*(cmd, resp) {
         cookie.host,
         cookie.name,
         cookie.path,
-        cookie.originAttributes,
+        false,
-        false);
+        cookie.originAttributes);
     return true;
   };
   this.mm.addMessageListener("Marionette:deleteCookie", cb);

toolkit/components/extensions/ext-cookies.js

@@ -295,7 +295,7 @@ extensions.registerSchemaAPI("cookies", "cookies", (extension, context) => {
 
       remove: function(details) {
         for (let cookie of query(details, ["url", "name", "storeId"], extension)) {
-          Services.cookies.remove(cookie.host, cookie.name, cookie.path, cookie.originAttributes, false);
+          Services.cookies.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes);
           // Todo: could there be multiple per subdomain?
           return Promise.resolve({
             url: details.url,

toolkit/components/extensions/test/mochitest/test_ext_cookies_permissions.html

@@ -117,7 +117,7 @@ function* testCookies(options) {
   yield extension.awaitMessage("change-cookies");
   cookieSvc.add(domain, "/", "x", "y", options.secure, false, false, options.expiry);
   cookieSvc.add(domain, "/", "x", "z", options.secure, false, false, options.expiry);
-  cookieSvc.remove(domain, "x", "/", {}, false);
+  cookieSvc.remove(domain, "x", "/", false, {});
   extension.sendMessage("cookies-changed");
 
   yield extension.awaitFinish("cookie-permissions");
@@ -168,7 +168,7 @@ function* testCookies(options) {
   }
 
   for (let cookie of cookies) {
-    cookieSvc.remove(cookie.host, cookie.name, "/", {}, false);
+    cookieSvc.remove(cookie.host, cookie.name, "/", false, {});
   }
   // Make sure we don't silently poison subsequent tests if something goes wrong.
   is(getCookies(options.domain).length, 0, "cookies cleared");

toolkit/forgetaboutsite/ForgetAboutSite.jsm

@@ -77,7 +77,7 @@ this.ForgetAboutSite = {
     let enumerator = cm.getCookiesFromHost(aDomain);
     while (enumerator.hasMoreElements()) {
       let cookie = enumerator.getNext().QueryInterface(Ci.nsICookie);
-      cm.remove(cookie.host, cookie.name, cookie.path, cookie.originAttributes, false);
+      cm.remove(cookie.host, cookie.name, cookie.path, false, cookie.originAttributes);
     }
 
     // EME

toolkit/mozapps/extensions/test/xpinstall/browser_cookies2.js

@@ -31,7 +31,7 @@ function finish_test(count) {
 
   var cm = Components.classes["@mozilla.org/cookiemanager;1"]
                      .getService(Components.interfaces.nsICookieManager2);
-  cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, {}, false);
+  cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, false, {});
 
   Services.perms.remove(makeURI("http://example.com"), "install");
 

toolkit/mozapps/extensions/test/xpinstall/browser_cookies3.js

@@ -33,7 +33,7 @@ function finish_test(count) {
 
   var cm = Components.classes["@mozilla.org/cookiemanager;1"]
                      .getService(Components.interfaces.nsICookieManager2);
-  cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, {}, false);
+  cm.remove("example.com", "xpinstall", "/browser/" + RELATIVE_DIR, false, {});
 
   Services.prefs.clearUserPref("network.cookie.cookieBehavior");
 

toolkit/mozapps/extensions/test/xpinstall/browser_cookies4.js

@@ -33,7 +33,7 @@ function finish_test(count) {
   is(count, 0, "No add-ons should have been installed");
   var cm = Components.classes["@mozilla.org/cookiemanager;1"]
                      .getService(Components.interfaces.nsICookieManager2);
-  cm.remove("example.org", "xpinstall", "/browser/" + RELATIVE_DIR, {}, false);
+  cm.remove("example.org", "xpinstall", "/browser/" + RELATIVE_DIR, false, {});
 
   Services.prefs.clearUserPref("network.cookie.cookieBehavior");
   Services.perms.remove(makeURI("http://example.com"), "install");