Add more data to net-internals view of DNS cache

chrome/browser/resources/net_internals/dns_view.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * This view displays information on the host resolver:
  *
  *   - Shows the default address family.
  *   - Shows the current host cache contents.
  *   - Has a button to clear the host cache.
@@ skipping 37 matching lines @@
   DnsView.INTERNAL_DNS_ENABLED_SPAN_ID = 'dns-view-internal-dns-enabled';
   DnsView.INTERNAL_DNS_INVALID_CONFIG_SPAN_ID =
       'dns-view-internal-dns-invalid-config';
   DnsView.INTERNAL_DNS_CONFIG_TBODY_ID = 'dns-view-internal-dns-config-tbody';
 
   DnsView.CLEAR_CACHE_BUTTON_ID = 'dns-view-clear-cache';
   DnsView.CAPACITY_SPAN_ID = 'dns-view-cache-capacity';
 
   DnsView.ACTIVE_SPAN_ID = 'dns-view-cache-active';
   DnsView.EXPIRED_SPAN_ID = 'dns-view-cache-expired';
+  DnsView.NETWORK_SPAN_ID = 'dns-view-network-changes';
   DnsView.CACHE_TBODY_ID = 'dns-view-cache-tbody';
 
   cr.addSingletonGetter(DnsView);
 
   DnsView.prototype = {
     // Inherit the superclass's methods.
     __proto__: superClass.prototype,
 
     onLoadLogFinish: function(data) {
       return this.onHostResolverInfoChanged(data.hostResolverInfo);
     },
 
     onHostResolverInfoChanged: function(hostResolverInfo) {
       // Clear the existing values.
       $(DnsView.CAPACITY_SPAN_ID).innerHTML = '';
       $(DnsView.CACHE_TBODY_ID).innerHTML = '';
       $(DnsView.ACTIVE_SPAN_ID).innerHTML = '0';
       $(DnsView.EXPIRED_SPAN_ID).innerHTML = '0';
+      $(DnsView.NETWORK_SPAN_ID).innerHTML = '0';
 
       // Update fields containing async DNS configuration information.
       displayAsyncDnsConfig_(hostResolverInfo);
 
       // No info.
       if (!hostResolverInfo || !hostResolverInfo.cache)
         return false;
 
       // Fill in the basic cache information.
       var hostResolverCache = hostResolverInfo.cache;
       $(DnsView.CAPACITY_SPAN_ID).innerText = hostResolverCache.capacity;
+      $(DnsView.NETWORK_SPAN_ID).innerText = hostResolverCache.network_changes;

[eroman, 2017/03/28 22:41:59]

When loading older log files, network_changes will not be defined, in which case
I imagine this will display "undefined".

How about replacing that expression with:

hostResolverCache.network_changes || '';

[mgersh, 2017/03/29 18:15:25]

Ah yeah. I thought to check old Chrome loading new netlog, but not new Chrome
loading old netlog. But your proposed simple fix doesn't work because 0 is
falsy, and properly getting this and the other fields to show empty string
instead of "undefined" seems like more complexity than it's worth in this
context for a case that won't happen much. What do you think?

[eroman, 2017/03/29 18:36:11]

Yes I think it is worth it.

In general not doing this can lead to artifacts like "undefined" or "null"
showing up when loading logs, or in other instances even worse it throws and
exception and then things break. It is often solved by adding checks accompanied
by a TODO comment that it can be removed in milestone YYY (a couple in the
future).

What do you think about something like this:

// TODO(yyy): The |ttl| and |network_changes| properties were introduced in M___
and may not exist when loading older logs. Can remove this in M___.
function valueOrDefault(value, defaultValue) {
  if (value != undefined)
    return value;
  return defaultValue
}

...innerText = valueOrDefault(hostResolverCache.network_changes, '');

and basically wrap each call to .ttl or network_changes in a call to
valueOrDefault(); should just be a couple callsites in this file, but does give
graceful backward compatibility.

[mgersh, 2017/03/29 20:20:53]

I still don't entirely understand why it's important for fields to be empty
instead of having the word "undefined" in them, but sure, done.

 
       var expiredEntries = 0;
       // Date the cache was logged.  This will be either now, when actively
       // logging data, or the date the log dump was created.
       var logDate;
       if (MainView.isViewingLoadedLog()) {
         logDate = new Date(ClientInfo.numericDate);
       } else {
         logDate = new Date();
       }
 
       // Fill in the cache contents table.
       for (var i = 0; i < hostResolverCache.entries.length; ++i) {
         var e = hostResolverCache.entries[i];
         var tr = addNode($(DnsView.CACHE_TBODY_ID), 'tr');
+        var expired = false;
 
         var hostnameCell = addNode(tr, 'td');
         addTextNode(hostnameCell, e.hostname);
 
         var familyCell = addNode(tr, 'td');
         addTextNode(familyCell, addressFamilyToString(e.address_family));
 
         var addressesCell = addNode(tr, 'td');
 
         if (e.error != undefined) {
           var errorText = e.error + ' (' + netErrorToString(e.error) + ')';
           var errorNode = addTextNode(addressesCell, 'error: ' + errorText);
           addressesCell.classList.add('warning-text');
         } else {
           addListToNode_(addNode(addressesCell, 'div'), e.addresses);
         }
 
+        var ttlCell = addNode(tr, 'td');
+        addTextNode(ttlCell, e.ttl);
+
         var expiresDate = timeutil.convertTimeTicksToDate(e.expiration);
         var expiresCell = addNode(tr, 'td');
         timeutil.addNodeWithDate(expiresCell, expiresDate);
         if (logDate > timeutil.convertTimeTicksToDate(e.expiration)) {
-          ++expiredEntries;
+          expired = true;
           var expiredSpan = addNode(expiresCell, 'span');
           expiredSpan.classList.add('warning-text');
           addTextNode(expiredSpan, ' [Expired]');
         }
+
+        var networkChangesCell = addNode(tr, 'td');
+        addTextNode(networkChangesCell, e.network_changes);
+        if (e.network_changes < hostResolverCache.network_changes) {

[eroman, 2017/03/28 22:41:59]

same comment -- what happens to this comparison when loading older logs that
don't have network_changes properties?


Also can you add a comment explaining this logic?

[mgersh, 2017/03/29 18:15:25]

undefined < undefined evaluates to false, so the comparison works fine as-is.
Done.

+          expired = true;
+          var expiredSpan = addNode(networkChangesCell, 'span');
+          expiredSpan.classList.add('warning-text');
+          addTextNode(expiredSpan, ' [Expired]');
+        }
+
+        if (expired) {
+          expiredEntries++;
+        }
       }
 
       $(DnsView.ACTIVE_SPAN_ID).innerText =
           hostResolverCache.entries.length - expiredEntries;
       $(DnsView.EXPIRED_SPAN_ID).innerText = expiredEntries;
       return true;
     },
   };
 
   /**
@@ skipping 49 matching lines @@
    * @param {DomNode} node The parent node.
    * @param {Array<string>} list List of strings to add to the node.
    */
   function addListToNode_(node, list) {
     for (var i = 0; i < list.length; ++i)
       addNodeWithText(node, 'div', list[i]);
   }
 
   return DnsView;
 })();