DevTools: clean up ObjectPropertiesSection.createValueElement

third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js

Index: third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
diff --git a/third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js b/third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
index 830e921d27f1532cc7c3da6b40c43d7f34ad9e2a..ede11f890768fa304f7a02d2f45ed24bfd47aac6 100644
--- a/third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
+++ b/third_party/WebKit/Source/devtools/front_end/components/ObjectPropertiesSection.js
@@ -241,63 +241,50 @@ Components.ObjectPropertiesSection = class extends TreeOutlineInShadow {
     var valueElement = createElementWithClass('span', 'value');
     var type = value.type;
     var subtype = value.subtype;
-    var description = value.description;
-    var prefix;
-    var valueText;
-    var suffix;
-    if (wasThrown) {
-      prefix = '[Exception: ';
-      valueText = description;
-      suffix = ']';
-    } else if (type === 'string' && typeof description === 'string') {
-      // Render \n as a nice unicode cr symbol.
-      prefix = '"';
-      valueText = description.replace(/\n/g, '\u21B5');
-      suffix = '"';
-    } else if (type !== 'object' || subtype !== 'node') {
-      valueText = description;
-    }
-
-    if (type === 'function') {
+    var description = value.description || '';

[dgozman, 2016/12/28 00:17:44]

Let's not do || ''

+    var isInternalLocation = type === 'object' && subtype === 'internal#location';
+    var isString = type === 'string' && typeof description === 'string';
+    var isNode = type === 'object' && subtype === 'node' && description;
+    var isNumberWithExponent = type === 'number' && description.indexOf('e') !== -1;

[dgozman, 2016/12/28 00:17:44]

Let's structure it differently:

var valueElement = null;

if (...)
  valueElement = createInternalLocationElement(...);
else if (...)
  valueElement = createStringElement(...);
...


if (!wasThrown)
  return valueElement;

wrap with exception and return

[luoe, 2017/01/03 21:52:31]

Done.

+    if (isInternalLocation) {
+      var rawLocation = value.debuggerModel().createRawLocationByScriptId(
+          value.value.scriptId, value.value.lineNumber, value.value.columnNumber);
+      // Return early if there is a linkified raw location.
+      if (rawLocation && linkifier)
+        return linkifier.linkifyRawLocation(rawLocation, '');
+      valueElement.textContent = '<unknown>';
+    } else if (isString) {
+      valueElement.createChild('span', 'object-value-string-quote').textContent = '"';
+      valueElement.createTextChild('').setTextContentTruncatedIfNeeded(description.replace(/\n/g, '\u21B5'));
+      valueElement.createChild('span', 'object-value-string-quote').textContent = '"';
+    } else if (type === 'function') {
       valueElement = Components.ObjectPropertiesSection.valueElementForFunctionDescription(description);
-    } else if (type !== 'number' || valueText.indexOf('e') === -1) {
-      valueElement.setTextContentTruncatedIfNeeded(valueText || '');
-      if (prefix)
-        valueElement.insertBefore(createTextNode(prefix), valueElement.firstChild);
-      if (suffix)
-        valueElement.createTextChild(suffix);
-    } else {
-      var numberParts = valueText.split('e');
-      var mantissa = valueElement.createChild('span', 'object-value-scientific-notation-mantissa');
-      mantissa.textContent = numberParts[0];
-      var exponent = valueElement.createChild('span', 'object-value-scientific-notation-exponent');
-      exponent.textContent = 'e' + numberParts[1];
+    } else if (isNode) {
+      Components.DOMPresentationUtils.createSpansForNodeTitle(valueElement, /** @type {string} */ (description));
+      valueElement.addEventListener('click', mouseClick, false);

[dgozman, 2016/12/28 00:17:44]

Inline the listeners.

[luoe, 2017/01/03 21:52:30]

Done.

+      valueElement.addEventListener('mousemove', mouseMove, false);
+      valueElement.addEventListener('mouseleave', mouseLeave, false);
+    } else if (isNumberWithExponent) {
+      var numberParts = description.split('e');
+      valueElement.createChild('span', 'object-value-scientific-notation-mantissa').textContent = numberParts[0];
+      valueElement.createChild('span', 'object-value-scientific-notation-exponent').textContent = 'e' + numberParts[1];
       valueElement.classList.add('object-value-scientific-notation-number');
       if (parentElement)  // FIXME: do it in the caller.
         parentElement.classList.add('hbox');
+    } else {
+      valueElement.setTextContentTruncatedIfNeeded(description);
     }
 
-    if (wasThrown)
-      valueElement.classList.add('error');
+    if (!isNode)
+      valueElement.title = description;

[dgozman, 2016/12/28 00:17:44]

Let's move these into the cases.

[luoe, 2017/01/03 21:52:30]

Done.

     if (subtype || type)
       valueElement.classList.add('object-value-' + (subtype || type));
-
-    if (type === 'object' && subtype === 'node' && description) {
-      Components.DOMPresentationUtils.createSpansForNodeTitle(valueElement, description);
-      valueElement.addEventListener('click', mouseClick, false);
-      valueElement.addEventListener('mousemove', mouseMove, false);
-      valueElement.addEventListener('mouseleave', mouseLeave, false);
-    } else {
-      valueElement.title = description || '';
-    }
-
-    if (type === 'object' && subtype === 'internal#location') {
-      var rawLocation = value.debuggerModel().createRawLocationByScriptId(
-          value.value.scriptId, value.value.lineNumber, value.value.columnNumber);
-      if (rawLocation && linkifier)
-        return linkifier.linkifyRawLocation(rawLocation, '');
-      valueElement.textContent = '<unknown>';
+    if (wasThrown) {

[dgozman, 2016/12/28 00:17:44]

Let's do at the start:
if (wasThrown) {
  valueElement.createTextChild('[Exception: ');
  valueElement = valueElement.createChild('span', 'value');
  valueElement.parentElement.createTextChild(']');
}

Let's add tests for 'throw function(){}', 'throw 123', 'throw "123"', 'throw new
Error("hi")'

[luoe, 2017/01/03 21:52:30]

Done.

+      valueElement.classList.add('error');
+      valueElement.insertBefore(createTextNode('[Exception: '), valueElement.firstChild);
+      valueElement.createTextChild(']');
     }
+    return valueElement;
 
     function mouseMove() {
       SDK.DOMModel.highlightObjectAsDOMNode(value);
@@ -314,8 +301,6 @@ Components.ObjectPropertiesSection = class extends TreeOutlineInShadow {
       Common.Revealer.reveal(value);
       event.consume(true);
     }
-
-    return valueElement;
   }
 
   /**