Bug 955603 - Displaying a large conversation log freezes the UI for a while before the first messages get displayed, r=aleth.
authorFlorian Quèze <florian@instantbird.org>
Fri, 13 Sep 2013 00:18:47 +0200
changeset 22776 008675d233ff81cfcb9c871b036b45c65c25bcfb
parent 22775 93fc5bbacff90c93fd4ac8ed136de7e9c84a3824
child 22777 9c110e087ddc1fb6ecc22c407e3f3a8d26334626
push id1225
push userflorian@queze.net
push dateSat, 11 Jan 2014 23:24:55 +0000
treeherdertry-comm-central@1d7aa08cb2d7 [default view] [failures only]
reviewersaleth
bugs955603
Bug 955603 - Displaying a large conversation log freezes the UI for a while before the first messages get displayed, r=aleth.
chat/components/public/imILogger.idl
chat/components/src/logger.js
chat/content/convbrowser.xml
im/content/viewlog.js
--- a/chat/components/public/imILogger.idl
+++ b/chat/components/public/imILogger.idl
@@ -27,16 +27,21 @@ interface imILogConversation: nsISupport
   // Other methods/attributes aren't implemented.
   readonly attribute imIAccount account;
 
   readonly attribute boolean isChat; // always false (compatibility with prplIConversation).
   readonly attribute imIAccountBuddy buddy; // always null (compatibility with prplIConvIM).
 
   void getMessages([optional] out unsigned long messageCount,
                    [retval, array, size_is(messageCount)] out prplIMessage messages);
+
+  // Callers that process the messages asynchronously should use the enumerator
+  // instead of the array version of the getMessages* methods to avoid paying
+  // up front the cost of xpconnect wrapping all message objects.
+  nsISimpleEnumerator getMessagesEnumerator([optional] out unsigned long messageCount);
 };
 
 [scriptable, uuid(164ff6c3-ca64-4880-b8f3-67eb1817955f)]
 interface imILog: nsISupports {
   readonly attribute AUTF8String path;
   readonly attribute PRTime time;
   readonly attribute AUTF8String format;
   // Will return null if the log format isn't json.
--- a/chat/components/src/logger.js
+++ b/chat/components/src/logger.js
@@ -346,20 +346,20 @@ function LogConversation(aLineInputStrea
       text: "",
       flags: ["noLog", "notification"]
     }
     if (!line.value) {
       // Bad log file.
       sessionMsg.text = bundle.formatStringFromName("badLogfile",
                                                     [inputStream.filename], 1);
       sessionMsg.flags.push("error", "system");
-      this._messages.push(new LogMessage(sessionMsg, this));
+      this._messages.push(sessionMsg);
       continue;
     }
-    this._messages.push(new LogMessage(sessionMsg, this));
+    this._messages.push(sessionMsg);
 
     if (firstFile) {
       let data = JSON.parse(line.value);
       this.startDate = new Date(data.date) * 1000;
       this.name = data.name;
       this.title = data.title;
       this._accountName = data.account;
       this._protocolName = data.protocol;
@@ -367,18 +367,17 @@ function LogConversation(aLineInputStrea
       firstFile = false;
     }
 
     while (more) {
       more = stream.readLine(line);
       if (!line.value)
         break;
       try {
-        let data = JSON.parse(line.value);
-        this._messages.push(new LogMessage(data, this));
+        this._messages.push(JSON.parse(line.value));
       } catch (e) {
         // if a message line contains junk, just ignore the error and
         // continue reading the conversation.
       }
     }
   }
 
   if (firstFile)
@@ -393,17 +392,30 @@ LogConversation.prototype = {
     name: this._accountName,
     normalizedName: this._accountName,
     protocol: {name: this._protocolName},
     statusInfo: Services.core.globalUserStatus
   }),
   getMessages: function(aMessageCount) {
     if (aMessageCount)
       aMessageCount.value = this._messages.length;
-    return this._messages;
+    return this._messages.map(function(m) new LogMessage(m, this), this);
+  },
+  getMessagesEnumerator: function(aMessageCount) {
+    if (aMessageCount)
+      aMessageCount.value = this._messages.length;
+    let enumerator = {
+      _index: 0,
+      _conv: this,
+      _messages: this._messages,
+      hasMoreElements: function() this._index < this._messages.length,
+      getNext: function() new LogMessage(this._messages[this._index++], this._conv),
+      QueryInterface: XPCOMUtils.generateQI([Ci.nsISimpleEnumerator])
+    };
+    return enumerator;
   }
 };
 
 /* Generic log enumeration stuff */
 function Log(aFile)
 {
   this.file = aFile;
   this.path = aFile.path;
--- a/chat/content/convbrowser.xml
+++ b/chat/content/convbrowser.xml
@@ -311,68 +311,103 @@
       <method name="appendMessage">
         <parameter name="aMsg"/>
         <parameter name="aContext"/>
         <parameter name="aFirstUnread"/>
         <body>
           <![CDATA[
             this._pendingMessages.push({msg: aMsg, context: aContext,
                                         firstUnread: aFirstUnread});
+            this.delayedDisplayPendingMessages();
+          ]]>
+        </body>
+      </method>
+
+      <method name="delayedDisplayPendingMessages">
+        <body>
+          <![CDATA[
             if (this._messageDisplayPending)
               return;
             this._messageDisplayPending = true;
             this.contentWindow.messageInsertPending = true;
             Services.tm.mainThread.dispatch(this.displayPendingMessages.bind(this),
                                             Ci.nsIEventTarget.DISPATCH_NORMAL);
           ]]>
         </body>
       </method>
 
       <field name="progressBar">null</field>
+
+      <!-- getNextPendingMessage and getPendingMessagesCount are the
+           only 2 methods accessing the this._pendingMessages array
+           directly during the chunked display of messages. It is
+           possible to override these 2 methods to replace the array
+           with something else. The log viewer for example uses an
+           enumerator that creates message objects lazily to avoid
+           jank when displaying lots of messages. -->
+      <!-- This variable is reset in onStateChange. -->
+      <field name="_nextPendingMessageIndex">0</field>
+      <method name="getNextPendingMessage">
+        <body>
+          <![CDATA[
+            if (this._nextPendingMessageIndex == this._pendingMessages.length) {
+              this._pendingMessages = [];
+              this._nextPendingMessageIndex = 0;
+              return null;
+            }
+            return this._pendingMessages[this._nextPendingMessageIndex++];
+          ]]>
+        </body>
+      </method>
+      <method name="getPendingMessagesCount">
+        <body>
+          <![CDATA[
+            return this._pendingMessages.length;
+          ]]>
+        </body>
+      </method>
+
       <!-- These variables are reset in onStateChange. -->
-      <field name="_nextPendingMessageIndex">0</field>
+      <field name="_pendingMessagesDisplayed">0</field>
       <field name="_displayPendingMessagesCalls">0</field>
       <method name="displayPendingMessages">
         <body>
           <![CDATA[
             if (!this._messageDisplayPending)
               return;
 
-            let max = this._pendingMessages.length;
-            let begin = Date.now();
-            let i;
-            for (i = this._nextPendingMessageIndex; i < max; ++i) {
-              let msg = this._pendingMessages[i];
-              this.displayMessage(msg.msg, msg.context, i + 1 < max,
+            let max = this.getPendingMessagesCount();
+            for (let begin = Date.now(); Date.now() - begin < 40; ) {
+              let msg = this.getNextPendingMessage();
+              if (!msg)
+                break;
+              this.displayMessage(msg.msg, msg.context,
+                                  ++this._pendingMessagesDisplayed < max,
                                   msg.firstUnread);
-              if (Date.now() - begin > 40)
-                break;
             }
             let event = document.createEvent("UIEvents");
             event.initUIEvent("MessagesDisplayed", false, false, window, 0);
             this.dispatchEvent(event);
-            if (i < max - 1) {
-              this._nextPendingMessageIndex = i + 1;
+            if (this._pendingMessagesDisplayed < max) {
               if (this.progressBar) {
                 // Show progress bar if after the third call (ca. 120ms)
                 // less than half the messages have been displayed.
                 if (++this._displayPendingMessagesCalls > 2 &&
-                    max > 2 * this._nextPendingMessageIndex)
+                    max > 2 * this._pendingMessagesDisplayed)
                   this.progressBar.hidden = false;
                 this.progressBar.max = max;
-                this.progressBar.value = this._nextPendingMessageIndex;
+                this.progressBar.value = this._pendingMessagesDisplayed;
               }
               Services.tm.mainThread.dispatch(this.displayPendingMessages.bind(this),
                                               Ci.nsIEventTarget.DISPATCH_NORMAL);
               return;
             }
             this.contentWindow.messageInsertPending = false;
             this._messageDisplayPending = false;
-            this._pendingMessages = [];
-            this._nextPendingMessageIndex = 0;
+            this._pendingMessagesDisplayed = 0;
             this._displayPendingMessagesCalls = 0;
             if (this.progressBar)
               this.progressBar.hidden = true;
           ]]>
         </body>
       </method>
 
       <!-- This is reset in onStateChange. -->
@@ -836,18 +871,21 @@
               // This can happen if the user quickly changes the selected
               // conversation in the log viewer.
               this._lastMessage = null;
               this._lastMessageIsContext = true;
               this._firstNonContextElt = null;
               this._messageDisplayPending = false;
               this._pendingMessages = [];
               this._nextPendingMessageIndex = 0;
+              this._pendingMessagesDisplayed = 0;
               this._displayPendingMessagesCalls = 0;
               this._sessions = [];
+              if (this.progressBar)
+                this.progressBar.hidden = true;
 
               Services.obs.notifyObservers(this, "conversation-loaded", null);
             }
           ]]>
         </body>
       </method>
 
       <method name="onProgressChange">
--- a/im/content/viewlog.js
+++ b/im/content/viewlog.js
@@ -131,21 +131,32 @@ var logWindow = {
     }
     return (this._colorCache[aName] = Math.round(res) % 360);
   },
   observe: function(aSubject, aTopic, aData) {
     let browser = document.getElementById("conv-browser");
     if (aTopic != "conversation-loaded" || aSubject != browser)
       return;
     browser._autoScrollEnabled = false;
-    for each (let msg in browser._conv.getMessages()) {
+
+    let count = {};
+    let messages = browser._conv.getMessagesEnumerator(count);
+    browser.getPendingMessagesCount = function() count.value;
+    browser.getNextPendingMessage = function() {
+      if (!messages.hasMoreElements()) {
+        delete browser.getNextPendingMessage;
+        return null;
+      }
+
+      let msg = messages.getNext();
       if (!msg.system && browser._conv.isChat)
-        msg.color = "color: hsl(" + this._computeColor(msg.who) + ", 100%, 40%);";
-      browser.appendMessage(msg);
-    }
+        msg.color = "color: hsl(" + logWindow._computeColor(msg.who) + ", 100%, 40%);";
+      return {msg: msg, context: false, firstUnread: false};
+    };
+    browser.delayedDisplayPendingMessages();
     delete this.pendingLoad;
     Services.obs.removeObserver(this, "conversation-loaded");
   },
 
   contentLoaded: function lw_contentLoaded() {
     let browser = document.getElementById("text-browser");
     if (browser.currentURI.spec == "about:blank")
       return;