Bug 1353533 - Don't create maps for non-session cookies when reloading all cookies r=mikedeboer
authorTim Taubert <ttaubert@mozilla.com>
Wed, 05 Apr 2017 16:05:46 +0200
changeset 351321 ec044ae4bb520ee7517388199a0ec7829d885704
parent 351320 7d360fe12f6d6b61ce4306c1f82b40791e46052c
child 351322 c6a73e6fcf5e26bc642d39623a3ebc3397666877
push id88837
push userttaubert@mozilla.com
push dateWed, 05 Apr 2017 14:11:44 +0000
treeherdermozilla-inbound@ec044ae4bb52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1353533
milestone55.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 1353533 - Don't create maps for non-session cookies when reloading all cookies r=mikedeboer When initializing the service in SessionCookies.jsm, SessionCookies._reloadCookies() currently iterates all cookies held by the coookie service and calls _updateCookie() to add them. _updateCookie() however is supposed to deal with cookie modifications, including session cookies being converted to longer-lived ones, and thus handles deletion too. This patch ensures a fast startup path by ignoring cookie deletion, we only ever need to add new session cookies when initializing on startup. It also changes the "cookie added" notification handler to ignore deletion. Additionally, CookieStore.delete() is a little more intelligent now and avoids the creation of maps only to find out the cookie didn't exist after all.
browser/components/sessionstore/SessionCookies.jsm
--- a/browser/components/sessionstore/SessionCookies.jsm
+++ b/browser/components/sessionstore/SessionCookies.jsm
@@ -127,16 +127,18 @@ var SessionCookiesInternal = {
 
   /**
    * Handles observers notifications that are sent whenever cookies are added,
    * changed, or removed. Ensures that the storage is updated accordingly.
    */
   observe(subject, topic, data) {
     switch (data) {
       case "added":
+        this._addCookie(subject);
+        break;
       case "changed":
         this._updateCookie(subject);
         break;
       case "deleted":
         this._removeCookie(subject);
         break;
       case "cleared":
         CookieStore.clear();
@@ -186,17 +188,28 @@ var SessionCookiesInternal = {
     if (entry.children) {
       for (let child of entry.children) {
         this._extractHostsFromEntry(child, hosts);
       }
     }
   },
 
   /**
-   * Updates or adds a given cookie to the store.
+   * Adds a given cookie to the store.
+   */
+  _addCookie(cookie) {
+    cookie.QueryInterface(Ci.nsICookie2);
+
+    if (cookie.isSession) {
+      CookieStore.set(cookie);
+    }
+  },
+
+  /**
+   * Updates a given cookie.
    */
   _updateCookie(cookie) {
     cookie.QueryInterface(Ci.nsICookie2);
 
     if (cookie.isSession) {
       CookieStore.set(cookie);
     } else {
       CookieStore.delete(cookie);
@@ -225,17 +238,17 @@ var SessionCookiesInternal = {
 
   /**
    * Iterates all cookies in the cookies service and puts them into the store
    * if they're session cookies.
    */
   _reloadCookies() {
     let iter = Services.cookies.enumerator;
     while (iter.hasMoreElements()) {
-      this._updateCookie(iter.getNext());
+      this._addCookie(iter.getNext());
     }
   }
 };
 
 /**
  * Generates all possible subdomains for a given host and prepends a leading
  * dot to all variants.
  *
@@ -378,17 +391,34 @@ var CookieStore = {
 
   /**
    * Removes a given cookie.
    *
    * @param cookie
    *        The nsICookie2 object to be removed from storage.
    */
   delete(cookie) {
-    this._ensureMap(cookie).delete(cookie.name);
+    // Deletion shouldn't create new maps.
+    // Bail out whenever we find that an entry or a map doesn't exist.
+    let map = this._hosts.get(cookie.host);
+    if (!map) {
+      return;
+    }
+
+    map = map.get(ChromeUtils.originAttributesToSuffix(cookie.originAttributes));
+    if (!map) {
+      return;
+    }
+
+    map = map.get(cookie.path);
+    if (!map) {
+      return;
+    }
+
+    map.delete(cookie.name);
   },
 
   /**
    * Removes all cookies.
    */
   clear() {
     this._hosts.clear();
   },