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..268468181f83cb5e550cbfdecf3255133e2fd975 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,14 @@ Console.ConsoleViewMessage = class {
     this._dataGrid = null;
     this._previewFormatter = new Components.RemoteObjectPreviewFormatter();
     this._searchRegex = null;
-    /** @type {?UI.Icon} */
-    this._messageLevelIcon = null;
+    /** @type {!UI.Icon|undefined} */
+    this._messageLevelIcon;
+    /** @type {!Element|undefined} */
+    this._nestingLevelMarkersElement;
+    /** @type {!Element|undefined} */
+    this._repeatCountElement;
+    /** @type {!Element|undefined} */
+    this._timestampElement;
   }
 
   /**
@@ -829,15 +835,15 @@ Console.ConsoleViewMessage = class {
   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)
+      if (!this._timestampElement) {
         this._timestampElement = createElementWithClass('span', 'console-timestamp');
+        this._insertDecorationsInOrder();
+      }
       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;
@@ -889,6 +895,19 @@ Console.ConsoleViewMessage = class {
     this._updateCloseGroupDecorations();
   }
 
+  _updateNestingLevelMarkers() {
+    if (!this._contentElement)
+      return;
+    if (!this._nestingLevelMarkersElement && this._nestingLevel > 0) {

[pfeldman, 2017/02/04 01:55:19]

The second time _updateNestingLevelMarkers is called, we already have
this._nestingLevelMarkersElement. And we add more and children into it, so it
grows indefinitely?

The way you cache things here is error-prone. I'd rather rebuild the element
upon update as it was previously.

Also, can nesting level change? It is different from other decorations in that
regard.

[luoe, 2017/02/06 19:51:55]

Nesting level doesn't change and messages don't grow indefinitely due to
grouping.

I've updated the semantics for almost all update()'s to clear its decoration
completely and rebuild contents as needed.

One exception is the repeat counter, which thrashes a ton unless the element is
reused.

+      this._nestingLevelMarkersElement = createElementWithClass('span', 'nesting-level-markers-wrapper');
+      this._nestingLevelMarkers = [];
+      for (var i = 0; i < this._nestingLevel; ++i)
+        this._nestingLevelMarkers.push(this._nestingLevelMarkersElement.createChild('div', 'nesting-level-marker'));
+      this._updateCloseGroupDecorations();
+      this._insertDecorationsInOrder();
+    }
+  }
+
   _updateCloseGroupDecorations() {
     if (!this._nestingLevelMarkers)
       return;
@@ -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,21 +998,55 @@ Console.ConsoleViewMessage = class {
       }
     }
 
+    // Update decorations after the content is generated.
     this._element.appendChild(this.contentElement());
-    if (this._repeatCount > 1)
-      this._showRepeatCountElement();
+    this.updateTimestamp();
+    this._updateMessageLevelIcon();

[pfeldman, 2017/02/04 01:55:19]

'create' these instead of 'updating'.

[luoe, 2017/02/06 19:51:55]

Timestamps and repeat counters can disappear after calling 'create', which is
confusing.  How about 'rebuild'?

+    this._updateNestingLevelMarkers();
+    this._updateRepeatCountElement();
   }
 
-  /**
-   * @param {string} iconType
-   */
-  _updateMessageLevelIcon(iconType) {
-    if (!iconType && !this._messageLevelIcon)
+  _insertDecorationsInOrder() {
+    if (!this._contentElement || this._contentElement.parentElement !== this._element)
+      return;
+    var leftMostElement = this._contentElement;
+    var addElementIfNeededBound = addElementIfNeeded.bind(this);
+    addElementIfNeededBound(this._repeatCountElement);

[pfeldman, 2017/02/04 01:55:19]

you can do 

[a, b, c].forEach(addElementIfNeeded.bind(this));

also, you are prepending, so you could name the method as such.

[luoe, 2017/02/06 19:51:55]

Acknowledged.

+    addElementIfNeededBound(this._nestingLevelMarkersElement);
+    addElementIfNeededBound(this._messageLevelIcon);
+    addElementIfNeededBound(this._timestampElement);
+
+    /**
+     * @this {!Console.ConsoleViewMessage}
+     * @param {!Element|undefined} elementToAdd
+     */
+    function addElementIfNeeded(elementToAdd) {
+      if (!elementToAdd)

[pfeldman, 2017/02/04 01:55:19]

Why are these nullable? We want inset to be the same at all time, so it seems
like they should present at least as the slots that maintain consistent
geometry.

[luoe, 2017/02/06 19:51:55]

Acknowledged.

+        return;
+      if (elementToAdd.parentElement !== this._element)
+        this._element.insertBefore(elementToAdd, leftMostElement);
+      leftMostElement = elementToAdd;
+    }
+  }
+
+  _updateMessageLevelIcon() {
+    if (!this._contentElement)
       return;
-    if (iconType && !this._messageLevelIcon) {
+    if (!this._messageLevelIcon) {
       this._messageLevelIcon = UI.Icon.create('', 'message-level-icon');
-      if (this._contentElement)
-        this._contentElement.insertBefore(this._messageLevelIcon, this._contentElement.firstChild);
+      this._insertDecorationsInOrder();
+    }
+
+    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._messageLevelIcon.setIconType(iconType);
   }
@@ -1016,26 +1060,22 @@ 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._updateRepeatCountElement();
   }
 
   incrementRepeatCount() {
     this._repeatCount++;
-    this._showRepeatCountElement();
+    this._updateRepeatCountElement();
   }
 
-  _showRepeatCountElement() {
+  _updateRepeatCountElement() {
     if (!this._contentElement)
       return;
-
-    if (!this._repeatCountElement) {
-      this._repeatCountElement = createElementWithClass('label', 'console-message-repeat-count', 'dt-small-bubble');
+    if (this._repeatCount > 1) {
+      if (!this._repeatCountElement) {
+        this._repeatCountElement = createElementWithClass('label', 'console-message-repeat-count', 'dt-small-bubble');
+        this._insertDecorationsInOrder();
+      }
       switch (this._message.level) {
         case SDK.ConsoleMessage.MessageLevel.Warning:
           this._repeatCountElement.type = 'warning';
@@ -1049,10 +1089,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 if (this._repeatCountElement) {
+      this._repeatCountElement.remove();
+      delete this._repeatCountElement;
     }
-    this._repeatCountElement.textContent = this._repeatCount;
+    if (this._element)
+      this._element.classList.toggle('repeated-message', this._repeatCount > 1);
   }
 
   get text() {