DevTools: insert console message decorations in order

third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js

Index: third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
diff --git a/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js b/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
index c4c5aea90db9c7cb8a8bd3cb1c525277f428f532..95bba4851dd37641fbcc8b98c71a5aeb8a28bfa6 100644
--- a/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
+++ b/third_party/WebKit/Source/devtools/front_end/console/ConsoleViewMessage.js
@@ -48,8 +48,10 @@ Console.ConsoleViewMessage = class {
     this._dataGrid = null;
     this._previewFormatter = new Components.RemoteObjectPreviewFormatter();
     this._searchRegex = null;
-    /** @type {?UI.Icon} */
-    this._messageLevelIcon = null;
+    /** @type {?Element} */
+    this._decorationWrapper = null;
+    /** @type {?Element} */
+    this._repeatCountElement = null;
   }
 
   /**
@@ -73,6 +75,7 @@ Console.ConsoleViewMessage = class {
   wasShown() {
     if (this._dataGrid)
       this._dataGrid.updateWidths();
+    this._buildDecorations();
     this._isVisible = true;
   }
 
@@ -91,6 +94,66 @@ Console.ConsoleViewMessage = class {
     this._cachedHeight = this.contentElement().offsetHeight;
   }
 
+  _buildDecorations() {

[pfeldman, 2017/02/07 18:50:29]

rebuildDecorations

[luoe, 2017/02/07 19:46:45]

Done.

+    this._decorationWrapper.removeChildren();
+
+    // Build timestamp.
+    var format = Common.moduleSetting('consoleTimestampFormat').get();
+    if (format !== Console.ConsoleViewMessage.TimestampFormat.None) {
+      var timestampElement = this._decorationWrapper.createChild('span', 'message-timestamp');
+      var timestampText = formatTimestamp(this._message.timestamp, format);
+      timestampElement.textContent = timestampText + ' ';
+      timestampElement.title = timestampText;
+    }
+
+    // Build level icon.
+    var iconType = '';
+    if (this._message.level === SDK.ConsoleMessage.MessageLevel.Warning)
+      iconType = 'smallicon-warning';
+    else if (this._message.level === SDK.ConsoleMessage.MessageLevel.Error)
+      iconType = 'smallicon-error';
+    else if (this._message.type === SDK.ConsoleMessage.MessageType.Command)
+      iconType = 'smallicon-user-command';
+    else if (this._message.type === SDK.ConsoleMessage.MessageType.Result)
+      iconType = 'smallicon-command-result';
+    this._decorationWrapper.appendChild(UI.Icon.create(iconType, 'message-level-icon'));
+
+    // Build nesting level markers.
+    this._nestingLevelMarkers = [];

[pfeldman, 2017/02/07 18:50:29]

You are now calling it on wasShown, I don't think _nestingLevelMarkers should be
mutated just because something was shown.

[luoe, 2017/02/07 19:46:45]

Yes, moved to updateMessageElement().

+    for (var i = 0; i < this._nestingLevel; ++i)
+      this._nestingLevelMarkers.push(this._decorationWrapper.createChild('div', 'message-nesting-level-marker'));
+    this._updateCloseGroupDecorations();
+
+    // Timestamps affect repeat count visibility.
+    this._rebuildRepeatCountElement();
+
+    /**
+     * @param {number} timestamp
+     * @param {!Console.ConsoleViewMessage.TimestampFormat} format
+     * @return {string}
+     */
+    function formatTimestamp(timestamp, format) {
+      var date = new Date(timestamp);
+      var yymmdd = date.getFullYear() + '-' + leadZero(date.getMonth() + 1, 2) + '-' + leadZero(date.getDate(), 2);
+      var hhmmssfff = leadZero(date.getHours(), 2) + ':' + leadZero(date.getMinutes(), 2) + ':' +
+          leadZero(date.getSeconds(), 2) + '.' + leadZero(date.getMilliseconds(), 3);
+      if (format === Console.ConsoleViewMessage.TimestampFormat.Full)
+        return yymmdd + ' ' + hhmmssfff;
+      return hhmmssfff;
+
+      /**
+       * @param {number} value
+       * @param {number} length
+       * @return {string}
+       */
+      function leadZero(value, length) {
+        var valueString = value.toString();
+        var padding = length - valueString.length;
+        return padding <= 0 ? valueString : '0'.repeat(padding) + valueString;
+      }
+    }
+  }
+
   /**
    * @return {number}
    */
@@ -826,50 +889,6 @@ Console.ConsoleViewMessage = class {
     return text.toLowerCase().includes(filter.toLowerCase());
   }
 
-  updateTimestamp() {
-    if (!this._contentElement)
-      return;
-
-    var format = Common.moduleSetting('consoleTimestampFormat').get();
-    if (format !== Console.ConsoleViewMessage.TimestampFormat.None) {
-      var timestampText = formatTimestamp(this._message.timestamp, format);
-      if (!this._timestampElement)
-        this._timestampElement = createElementWithClass('span', 'console-timestamp');
-      this._timestampElement.textContent = timestampText + ' ';
-      this._timestampElement.title = timestampText;
-      this._contentElement.insertBefore(this._timestampElement, this._contentElement.firstChild);
-    } else if (this._timestampElement) {
-      this._timestampElement.remove();
-      delete this._timestampElement;
-    }
-
-    /**
-     * @param {number} timestamp
-     * @param {!Console.ConsoleViewMessage.TimestampFormat} format
-     * @return {string}
-     */
-    function formatTimestamp(timestamp, format) {
-      var date = new Date(timestamp);
-      var yymmdd = date.getFullYear() + '-' + leadZero(date.getMonth() + 1, 2) + '-' + leadZero(date.getDate(), 2);
-      var hhmmssfff = leadZero(date.getHours(), 2) + ':' + leadZero(date.getMinutes(), 2) + ':' +
-          leadZero(date.getSeconds(), 2) + '.' + leadZero(date.getMilliseconds(), 3);
-      if (format === Console.ConsoleViewMessage.TimestampFormat.Full)
-        return yymmdd + ' ' + hhmmssfff;
-      return hhmmssfff;
-
-      /**
-       * @param {number} value
-       * @param {number} length
-       * @return {string}
-       */
-      function leadZero(value, length) {
-        var valueString = value.toString();
-        var padding = length - valueString.length;
-        return padding <= 0 ? valueString : '0'.repeat(padding) + valueString;
-      }
-    }
-  }
-
   /**
    * @return {number}
    */
@@ -906,12 +925,12 @@ Console.ConsoleViewMessage = class {
       return this._contentElement;
 
     var contentElement = createElementWithClass('div', 'console-message');
-    if (this._messageLevelIcon)
-      contentElement.appendChild(this._messageLevelIcon);
     this._contentElement = contentElement;
     if (this._message.type === SDK.ConsoleMessage.MessageType.StartGroup ||
         this._message.type === SDK.ConsoleMessage.MessageType.StartGroupCollapsed)
       contentElement.classList.add('console-group-title');
+    else if (this._message.type === SDK.ConsoleMessage.MessageType.Result)
+      contentElement.classList.add('console-user-command-result');
 
     var formattedMessage;
     var consoleMessage = this._message;
@@ -929,7 +948,6 @@ Console.ConsoleViewMessage = class {
       formattedMessage = this._buildMessage(consoleMessage);
     contentElement.appendChild(formattedMessage);
 
-    this.updateTimestamp();
     return this._contentElement;
   }
 
@@ -951,28 +969,20 @@ Console.ConsoleViewMessage = class {
 
     this._element.className = 'console-message-wrapper';
     this._element.removeChildren();
-
-    this._nestingLevelMarkers = [];
-    for (var i = 0; i < this._nestingLevel; ++i)
-      this._nestingLevelMarkers.push(this._element.createChild('div', 'nesting-level-marker'));
-    this._updateCloseGroupDecorations();
     this._element.message = this;
 
     switch (this._message.level) {
       case SDK.ConsoleMessage.MessageLevel.Verbose:
         this._element.classList.add('console-verbose-level');
-        this._updateMessageLevelIcon('');
         break;
       case SDK.ConsoleMessage.MessageLevel.Info:
         this._element.classList.add('console-info-level');
         break;
       case SDK.ConsoleMessage.MessageLevel.Warning:
         this._element.classList.add('console-warning-level');
-        this._updateMessageLevelIcon('smallicon-warning');
         break;
       case SDK.ConsoleMessage.MessageLevel.Error:
         this._element.classList.add('console-error-level');
-        this._updateMessageLevelIcon('smallicon-error');
         break;
     }
 
@@ -988,23 +998,9 @@ Console.ConsoleViewMessage = class {
       }
     }
 
+    this._decorationWrapper = this._element.createChild('span', 'message-decoration-wrapper');
+    this._repeatCountElement = this._element.createChild('label', 'message-repeat-count', 'dt-small-bubble');
     this._element.appendChild(this.contentElement());
-    if (this._repeatCount > 1)
-      this._showRepeatCountElement();
-  }
-
-  /**
-   * @param {string} iconType
-   */
-  _updateMessageLevelIcon(iconType) {
-    if (!iconType && !this._messageLevelIcon)
-      return;
-    if (iconType && !this._messageLevelIcon) {
-      this._messageLevelIcon = UI.Icon.create('', 'message-level-icon');
-      if (this._contentElement)
-        this._contentElement.insertBefore(this._messageLevelIcon, this._contentElement.firstChild);
-    }
-    this._messageLevelIcon.setIconType(iconType);
   }
 
   /**
@@ -1016,26 +1012,20 @@ Console.ConsoleViewMessage = class {
 
   resetIncrementRepeatCount() {
     this._repeatCount = 1;
-    if (!this._repeatCountElement)
-      return;
-
-    this._repeatCountElement.remove();
-    if (this._contentElement)
-      this._contentElement.classList.remove('repeated-message');
-    delete this._repeatCountElement;
+    this._rebuildRepeatCountElement();
   }
 
   incrementRepeatCount() {
     this._repeatCount++;
-    this._showRepeatCountElement();
+    this._rebuildRepeatCountElement();
   }
 
-  _showRepeatCountElement() {
-    if (!this._contentElement)
+  _rebuildRepeatCountElement() {

[pfeldman, 2017/02/07 18:50:29]

That is now _updateRepeatCountElement, could we also inline it in the same
decorations method? Then we just call updateMessage on the one that has repeat
changed.

[luoe, 2017/02/07 19:46:45]

Renamed.

I don't think we can safely inline it, the callers resetIncrementRepeatCount()
and incrementRepeatCount() cannot safely call anything that removes and adds
things to the DOM.

What we can do is move the switch logic into updateMessageElement(), since it
doesn't change when the counter increments.

+    // Repeat counter may be updated before a message is rendered.
+    if (!this._repeatCountElement)
       return;
-
-    if (!this._repeatCountElement) {
-      this._repeatCountElement = createElementWithClass('label', 'console-message-repeat-count', 'dt-small-bubble');
+    var show = this._repeatCount > 1;
+    if (show) {
       switch (this._message.level) {
         case SDK.ConsoleMessage.MessageLevel.Warning:
           this._repeatCountElement.type = 'warning';
@@ -1049,10 +1039,13 @@ Console.ConsoleViewMessage = class {
         default:
           this._repeatCountElement.type = 'info';
       }
-      this._element.insertBefore(this._repeatCountElement, this._contentElement);
-      this._contentElement.classList.add('repeated-message');
+      this._repeatCountElement.textContent = this._repeatCount;
+    } else {
+      this._repeatCountElement.textContent = '';
     }
-    this._repeatCountElement.textContent = this._repeatCount;
+    this._repeatCountElement.classList.toggle('hidden', !show);
+    if (this._element)
+      this._element.classList.toggle('repeated-message', show);
   }
 
   get text() {