Bug 1652279 Comment 23 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

># HG changeset patch
># User Alessandro Castellani <alessandro@thunderbird.net>
># Date 1595370029 25200
>#      Tue Jul 21 15:20:29 2020 -0700
># Node ID 9adb9b8d780701d1affeab7285c882e0e3b55ba5
># Parent  ab1568fa6f2b090e1b6c2a757f8e353a90365233
>Bug 1652279 - Fix startup performance issues with folder colour customization. r=?
>
>diff --git a/mail/base/content/folderPane.js b/mail/base/content/folderPane.js
>--- a/mail/base/content/folderPane.js
>+++ b/mail/base/content/folderPane.js
>@@ -176,17 +176,19 @@ var gFolderTreeView = {
>       // This is ok.  If we've already migrated we'll end up here
>     }
> 
>     if (aJSONFile) {
>       // Parse our persistent-open-state json file
>       let data = IOUtils.loadFileToString(aJSONFile);
>       if (data) {
>         try {
>-          this._persistOpenMap = JSON.parse(data);
>+          let parsedData = JSON.parse(data);
>+          this._persistOpenMap = parsedData[0];
>+          this._persistColorMap = parsedData[1];
>         } catch (x) {
>           Cu.reportError(
>             gFolderTreeView.messengerBundle.getFormattedString(
>               "failedToReadFile",
>               [aJSONFile, x]
>             )
>           );
>         }
>@@ -203,26 +205,26 @@ var gFolderTreeView = {
>     this.toggleCols(true);
>     gFolderStatsHelpers.init();
> 
>     // Add this listener so that we can update the tree when things change
>     MailServices.mailSession.AddFolderListener(this, Ci.nsIFolderListener.all);
>   },
> 
>   /**
>-   * Called when the window is being torn down.  Here we undo everything we did
>-   * onload.  That means removing our listener and serializing our JSON.
>+   * Called when the window is being torn down. Here we undo everything we did
>+   * onload. That means removing our listener and serializing our JSON.
>    */
>   unload(aJSONFile) {
>     // Remove our listener
>     MailServices.mailSession.RemoveFolderListener(this);
> 
>     if (aJSONFile) {
>       // Write out our json file...
>-      let data = JSON.stringify(this._persistOpenMap);
>+      let data = JSON.stringify([this._persistOpenMap, this._persistColorMap]);
>       IOUtils.saveStringToFile(aJSONFile, data);
>     }
>   },
> 
>   /**
>    * Extensions can use this function to add a new mode to the folder pane.
>    *
>    * @param aCommonName  an internal name to identify this mode. Must be unique
>@@ -1273,21 +1275,16 @@ var gFolderTreeView = {
>       let folder = this._rowMap[aIndex]._folder;
>       if (aExpandServer) {
>         if (folder.isServer) {
>           folder.server.performExpand(msgWindow);
>         } else if (folder instanceof Ci.nsIMsgImapMailFolder) {
>           folder.performExpand(msgWindow);
>         }
>       }
>-
>-      // Restore the custom color of the folder icon for the available children.
>-      for (let child of Array.from(this._rowMap[aIndex].children)) {
>-        this.setFolderCustomColor(child._folder);
>-      }
>     }
>   },
> 
>   _subFoldersWithStringProperty(folder, folders, aFolderName, deep) {
>     for (let child of fixIterator(folder.subFolders, Ci.nsIMsgFolder)) {
>       // if the folder selection is based on a string property, use that
>       if (aFolderName == getSmartFolderName(child)) {
>         folders.push(child);
>@@ -1532,29 +1529,36 @@ var gFolderTreeView = {
>     "favorite",
>     "favorite_compact",
>     "recent_compact",
>     "smart",
>   ],
>   _modeDisplayNames: {},
> 
>   /**
>-   * This is a javascript map of which folders we had open, so that we can
>-   * persist their state over-time.  It is designed to be used as a JSON object.
>+   * This is a JavaScript map of which folders we had open, so that we can
>+   * persist their state over-time. It is designed to be used as a JSON object.
>    */
>   _persistOpenMap: {},
>   _notPersistedModes: [
>     "unread",
>     "unread_compact",
>     "favorite",
>     "favorite_compact",
>     "recent_compact",
>   ],
> 
>   /**
>+   * This is a JavaScript map of which folders have a custom color so that we
>+   * can persist the customization state over-time. It is designed to be used
>+   * as a JSON object.
>+   */
>+  _persistColorMap: [],
>+
>+  /**
>    * Iterate over the persistent list and open the items (folders) stored in it.
>    */
>   _restoreOpenStates() {
>     let mode = this.mode;
>     // Remove any saved state of modes where open state should not be persisted.
>     // This is mostly for migration from older profiles that may have the info stored.
>     if (this._notPersistedModes.includes(mode)) {
>       delete this._persistOpenMap[mode];
>@@ -1569,19 +1573,16 @@ var gFolderTreeView = {
>       // So fallback on old trick of going backwards from the end, which
>       // doesn't care when you add things at the end.
>       for (let i = tree._rowMap.length - 1; i >= 0; i--) {
>         let row = tree._rowMap[i];
>         if (row.level != curLevel) {
>           continue;
>         }
> 
>-        // Restore the custom color of the folder icon.
>-        tree.setFolderCustomColor(row._folder);
>-
>         // The initial state of all rows is closed, so toggle those we want open.
>         if (!map || map.includes(row.id)) {
>           tree._toggleRow(i, false);
>           goOn = true;
>         }
>       }
> 
>       // If we opened up any new kids, we need to check their level as well.
>@@ -1589,16 +1590,101 @@ var gFolderTreeView = {
>       if (goOn) {
>         openLevel();
>       }
>     }
>     openLevel();
>   },
> 
>   /**
>+   * Iterate over the custom color list and apply the CSS style stored in it.
>+   */
>+  _restoreCustomColors() {
>+    let tree = this;
>+    let map = tree._persistColorMap;
>+
>+    // Interrrupt if the user never defined any custom color.
>+    if (!map) {
>+      return;
>+    }
>+
>+    // Loop through all the saved folders and restore their colors.
>+    for (let { color, id } of map) {
>+      // Store the color in the cache property so we can use this for
>+      // properties changes and updates.
>+      gFolderTreeView.setFolderCacheProperty(
>+        {
>+          URI: id,
>+        },
>+        "folderIconColor",
>+        color
>+      );
>+
>+      // Append the color to the inline CSS.
>+      this.appendColor(color);
>+    }
>+  },
>+
>+  /**
>+   * Remove the item from the persisted list of custom colored folder.
>+   *
>+   * @param {string} folderId - The URI of the folder item.
>+   */
>+  _removeCustomColor(folderId) {
>+    // Interrupt if the map hasn't been defined.
>+    if (!this._persistColorMap) {
>+      return;
>+    }
>+
>+    let index = this._persistColorMap.findIndex(item => item.id === folderId);
>+    if (index != -1) {
>+      this._persistColorMap.splice(index, 1);
>+    }
>+  },
>+
>+  /**
>+   * Add the item to the persisted list of custom colored folder.
>+   *
>+   * @param {string} folderId - The URI of the folder item.
>+   * @param {string} color - The selected custom color.
>+   */
>+  _addCustomColor(folderId, color) {
>+    // Always remove the previous color if it exists.
>+    this._removeCustomColor(folderId);
>+
>+    // Interrupt if no custom color was defined.
>+    if (!color) {
>+      return;
>+    }
>+
>+    // Create the map if is undefined.
>+    if (!this._persistColorMap) {
>+      this._persistColorMap = [];
>+    }
>+
>+    this._persistColorMap.push({
>+      id: folderId,
>+      color,
>+    });
>+
>+    // Store the color in the cache property so we can use this for
>+    // properties changes and updates.
>+    gFolderTreeView.setFolderCacheProperty(
>+      {
>+        URI: folderId,
>+      },
>+      "folderIconColor",
>+      color
>+    );
>+
>+    // Append the color to the inline CSS.
>+    this.appendColor(color);
>+  },
>+
>+  /**
>    * Remove the item from the persistent list, meaning the item should
>    * be persisted as closed in the tree.
>    *
>    * @param aItemId  The URI of the folder item.
>    */
>   _persistItemClosed(aItemId) {
>     let mode = this.mode;
>     if (this._notPersistedModes.includes(mode)) {
>@@ -1678,16 +1764,18 @@ var gFolderTreeView = {
> 
>     if (this._tree) {
>       if (oldCount !== null) {
>         this._tree.rowCountChanged(0, this._rowMap.length - oldCount);
>       }
>       this._tree.invalidate();
>     }
>     this._restoreOpenStates();
>+    this._restoreCustomColors();
>+
>     // restore selection.
>     for (let folder of selectedFolders) {
>       if (folder) {
>         let index = this.getIndexOfFolder(folder);
>         if (index != null) {
>           this.selection.toggleSelect(index);
>         }
>       }
>@@ -2596,65 +2684,16 @@ var gFolderTreeView = {
>   OnItemEvent(aFolder, aEvent) {
>     let index = this.getIndexOfFolder(aFolder);
>     if (index != null) {
>       this._tree.invalidateRow(index);
>     }
>   },
> 
>   /**
>-   * Apply custom icon colors if a cached property is not already present.
>-   *
>-   * @param {ftvItem} folder - The folder attached to this row in the tree.
>-   */
>-  setFolderCustomColor(folder) {
>-    // Interrupt if the folder already has an icon color cached property.
>-    if (gFolderTreeView.getFolderCacheProperty(folder, "folderIconColor")) {
>-      return;
>-    }
>-
>-    let msgDatabase;
>-    try {
>-      // This will throw an exception if the .msf file is missing,
>-      // out of date (e.g., the local folder has changed), or corrupted.
>-      msgDatabase = folder.msgDatabase;
>-    } catch (e) {}
>-
>-    // Interrupt if no folder database is available.
>-    if (!msgDatabase) {
>-      return;
>-    }
>-
>-    // Get the previously stored color from the Folder Database.
>-    let iconColor = msgDatabase.dBFolderInfo.getCharProperty("folderIconColor");
>-
>-    // Interrupt if no custom color was defined.
>-    if (!iconColor) {
>-      return;
>-    }
>-
>-    // Store the color in the cache property so we can use this for
>-    // properties changes and updates.
>-    gFolderTreeView.setFolderCacheProperty(
>-      folder,
>-      "folderIconColor",
>-      iconColor
>-    );
>-    this.appendColor(iconColor);
>-
>-    // Null out to avoid memory bloat.
>-    if (
>-      !MailServices.mailSession.IsFolderOpenInWindow(folder) &&
>-      !(folder.flags & (Ci.nsMsgFolderFlags.Trash | Ci.nsMsgFolderFlags.Inbox))
>-    ) {
>-      folder.msgDatabase = null;
>-    }
>-  },
>-
>-  /**
>    * Append inline CSS style for those icons where a custom color was defined.
>    *
>    * @param {string} iconColor - The hash color.
>    */
>   appendColor(iconColor) {
>     if (!this.folderColorStyle || !iconColor) {
>       return;
>     }
>@@ -3421,16 +3460,19 @@ var gFolderTreeController = {
>   previewSelectedColor(folder, newColor) {
>     // If the color is null, it means we're resetting to the default value.
>     if (!newColor) {
>       gFolderTreeView.setFolderCacheProperty(folder, "folderIconColor", "");
> 
>       // Clear the preview CSS.
>       gFolderTreeView.folderColorPreview.textContent = "";
> 
>+      // Remove the stored value from the json map if present.
>+      gFolderTreeView._removeCustomColor(folder.URI);
>+
>       // Force the folder update to see the new color.
>       gFolderTreeView._tree.invalidateRow(
>         gFolderTreeView.getIndexOfFolder(folder)
>       );
>       return;
>     }
> 
>     // Add the new color property.
>@@ -3456,24 +3498,18 @@ var gFolderTreeController = {
>     // Clear the preview CSS.
>     gFolderTreeView.folderColorPreview.textContent = "";
> 
>     let newColor = gFolderTreeView.getFolderCacheProperty(
>       folder,
>       "folderIconColor"
>     );
> 
>-    // Append new inline color if defined.
>-    gFolderTreeView.appendColor(newColor);
>-
>-    // Store the new color in the Folder database.
>-    folder.msgDatabase.dBFolderInfo.setCharProperty(
>-      "folderIconColor",
>-      newColor
>-    );
>+    // Store the new color in the json map.
>+    gFolderTreeView._addCustomColor(folder.URI, newColor);
> 
>     // Force the folder update to set the new color.
>     gFolderTreeView._tree.invalidateRow(
>       gFolderTreeView.getIndexOfFolder(folder)
>     );
>   },
> 
>   /**

Back to Bug 1652279 Comment 23