Change chrome://net-export/ UI flow.

components/net_log/resources/net_export.js

Index: components/net_log/resources/net_export.js
diff --git a/components/net_log/resources/net_export.js b/components/net_log/resources/net_export.js
index 3ea73c575b65bffa1dafa1187a44ac01e5ce807b..7d2c26144001162806c6d78d46bdbc25f0c03e4f 100644
--- a/components/net_log/resources/net_export.js
+++ b/components/net_log/resources/net_export.js
@@ -12,7 +12,7 @@ function onLoad() {
 document.addEventListener('DOMContentLoaded', onLoad);
 
 /**
- * This class handles the presentation of our profiler view. Used as a
+ * This class handles the presentation of the net-export view. Used as a
  * singleton.
  */
 var NetExportView = (function() {
@@ -20,18 +20,30 @@ var NetExportView = (function() {
 
   // --------------------------------------------------------------------------
 
+  var kIdStateDivUninitialized = 'export-view-state-uninitialized';
+  var kIdStateDivInitial = 'export-view-state-initial';
+  var kIdStateDivLogging = 'export-view-state-logging';
+  var kIdStateDivStopped = 'export-view-state-stopped';
+  var kIdStartLoggingButton = 'export-view-start-logging';
+  var kIdStopLoggingButton = 'export-view-stop-logging';
+  var kIdEmailLogButton = 'export-view-mobile-email';
+  var kIdCaptureModeLogging = 'export-view-capture-mode-logging';
+  var kIdFilePathLogging = 'export-view-file-path-logging';
+  var kIdCaptureModeStopped = 'export-view-capture-mode-stopped';
+  var kIdFilePathStoppedLogging = 'export-view-file-path-stopped';
+  var kIdStartOverButton = 'export-view-startover';
+  var kIdReadMoreLink = 'export-view-privacy-read-more-link';
+  var kIdReadMoreDiv = 'export-view-privacy-read-more'

[xunjieli, 2017/05/02 17:10:01]

Do these elements need an "export-view" prefix? The names are rather long. If we
can get rid of "export-view" prefix, it's easier to read and correlate the
values in the CSS file with those in the HTML file.
I noticed that the new elements that you added ("section-container" and
"outline-box") do not have this prefix, so i guess the prefix is not strictly
needed.

[eroman, 2017/05/02 20:07:58]

Done.

+
   /**
    * @constructor
    */
   function NetExportView() {
-    $('export-view-start-data').onclick = this.onStartData_.bind(this);
-    $('export-view-stop-data').onclick = this.onStopData_.bind(this);
-    if (this.useMobileUI_())
-      $('export-view-mobile-send-data').onclick = this.onSendData_.bind(this);
-
     // Tell NetExportMessageHandler to notify the UI of future state changes
     // from this point on (through onExportNetLogInfoChanged()).
     chrome.send('enableNotifyUIWithState');
+
+    this.infoForLoggedFile_ = null;
   }
 
   cr.addSingletonGetter(NetExportView);
@@ -40,7 +52,7 @@ var NetExportView = (function() {
     /**
      * Starts saving NetLog data to a file.
      */
-    onStartData_: function() {
+    onStartLogging_: function() {
       var logMode =
           document.querySelector('input[name="log-mode"]:checked').value;
       chrome.send('startNetLog', [logMode]);
@@ -49,85 +61,190 @@ var NetExportView = (function() {
     /**
      * Stops saving NetLog data to a file.
      */
-    onStopData_: function() {
+    onStopLogging_: function() {
       chrome.send('stopNetLog');
     },
 
     /**
-     * Sends NetLog data via email from browser.
+     * Sends NetLog data via email from browser (mobile only)

[xunjieli, 2017/05/02 17:10:01]

nit: add a period at the end.

[eroman, 2017/05/02 20:07:58]

Done.

      */
-    onSendData_: function() {
+    onSendEmail_: function() {
       chrome.send('sendNetLog');
     },
 
     /**
-     * Updates the UI to reflect the current state. Displays the path name of
-     * the file where NetLog data is collected.
+     * Transitions back to the "Start logging to disk" state.
      */
-    onExportNetLogInfoChanged: function(exportNetLogInfo) {
-      if (exportNetLogInfo.file) {
-        var message = '';
-        if (exportNetLogInfo.state == 'LOGGING')
-          message = 'NetLog data is collected in: ';
-        else if (exportNetLogInfo.logType != 'NONE')
-          message = 'NetLog data to send is in: ';
-        $('export-view-file-path-text').textContent =
-            message + exportNetLogInfo.file;
-      } else {
-        $('export-view-file-path-text').textContent = '';
-      }
+    onStartOver_: function() {
+      this.infoForLoggedFile_ = null;
+      this.renderInitial_();
+    },
 
-      // Disable all controls.  Useable controls are enabled below.
-      var controls = document.querySelectorAll('button, input');
-      for (var i = 0; i < controls.length; ++i) {
-        controls[i].disabled = true;
+    /**
+     * Updates the UI to reflect the current state. The state transitions are
+     * sent by the browser controller (NetLogFileWriter):
+     *
+     *   * UNINITIALIZED - This is the initial state when net-export is opened
+     *         for the first time, or there was an error during initialization.
+     *         This state is short-lived and likely not observed; will
+     *         immediately transition to INITIALIZING).
+     *
+     *   * INITIALIZING - On desktop UI this is pretty much a no-op. On the
+     *         mobile UI, this state is when the controller checks the disk for
+     *         a previous net-log file (from past run of the browser). After
+     *         success will transition to NOT_LOGGING. On failure will
+     *         transition to UNINITIALIZED (rare).
+     *
+     *   * NOT_LOGGING - This is the steady-state. It means initialization
+     *        completed and we are not currently logging. Being in this state
+     *        either means:
+     *         (1) We were logging and then the user stopped (earlier states
+     *             were STATE_LOGGING / STATE_STOPPING_LOG).
+     *         (2) We have never started logging (previous state was
+     *             INITIALIZING).
+     *
+     *   * STARTING_LOG - This state means the user has clicked the "Start log"
+     *        button and logging is about to get started (files may not have
+     *        been created yet).
+     *
+     *   * LOGGING - This state means that logging is currently in progress.
+     *        The destination path of the log, and the capture mode are known
+     *        and will be reflected in the parameters.
+     *
+     *   * STOPPING_LOG - This state means the user has clicked the "Stop
+     *        logging" button, and the log file is in the process of being
+     *        finalized. Once the state transitions to NOT_LOGGING then the log
+     *        is complete, and can safely be copied/emailed.
+     */
+    onExportNetLogInfoChanged: function(info) {
+      switch (info.state) {
+        case 'UNINITIALIZED':
+        case 'INITIALIZING':
+          this.renderUninitialized_();
+          break;
+
+        case 'NOT_LOGGING':
+          if (this.infoForLoggedFile_) {
+            // There is no "stopped logging" state. We manufacture that in the
+            // UI in response to a transition from LOGGING --> NOT_LOGGING.
+            this.renderStoppedLogging_(this.infoForLoggedFile_);
+
+            // TODO(eroman): prevent future state transitions. In desktop UI
+            // could start logging in a new tab, and it would reset this one.
+          } else if (info.logExists) {
+            // In the mobile UI, initialization may have found a
+            // pre-existing log file.
+            this.renderStoppedLogging_(info);
+          } else {
+            this.renderInitial_(info);
+          }
+          break;
+
+        case 'STARTING_LOG':
+          // This is a short-lived state, no need to do anything special.
+          // Disabling the buttons would be nice, however it is not crucial as
+          // the controller will reject commands while in this state anyway.
+          this.renderInitial_(info);
+          break;
+
+        case 'LOGGING':
+          // Cache the last information for this logging session, so once
+          // logging is stopped will know what path information to display.
+          this.infoForLoggedFile_ = info;
+          this.renderLogging_(info);
+          break;
+
+        case 'STOPPING_LOG':
+          // This is a short-lived state, no need to do anything special.
+          this.renderLogging_(info);
+          break;
       }
+    },
 
-      if (this.useMobileUI_()) {
-        $('export-view-mobile-deletes-log-text').hidden = true;
-        $('export-view-mobile-private-data-text').hidden = true;
-        $('export-view-mobile-send-old-log-text').hidden = true;
-      }
+    /**
+     * Updates the UI to display the "uninitialized" state. This is only
+     * visible for a short period of time, or longer if initialization failed
+     * (and didn't transition to a different state).
+     */
+    renderUninitialized_: function(info) {

[xunjieli, 2017/05/02 17:10:01]

Why do we need to pass |info| here and for renderInitial_? If it's not needed,
maybe we could eliminate the argument?

[eroman, 2017/05/02 20:07:58]

Done.

+      this.showStateDiv_(kIdStateDivUninitialized);
+    },
 
-      if (exportNetLogInfo.state == 'NOT_LOGGING') {
-        // Allow making a new log.
-        $('export-view-strip-private-data-button').disabled = false;
-        $('export-view-include-private-data-button').disabled = false;
-        $('export-view-log-bytes-button').disabled = false;
-        $('export-view-start-data').disabled = false;
-
-        // If there's a pre-existing log, allow sending it (this only
-        // applies to the mobile UI).
-        if (this.useMobileUI_() && exportNetLogInfo.logExists) {
-          $('export-view-mobile-deletes-log-text').hidden = false;
-          $('export-view-mobile-send-data').disabled = false;
-          if (!exportNetLogInfo.logCaptureModeKnown) {
-            $('export-view-mobile-send-old-log-text').hidden = false;
-          } else if (exportNetLogInfo.captureMode != 'STRIP_PRIVATE_DATA') {
-            $('export-view-mobile-private-data-text').hidden = false;
-          }
-        }
-      } else if (exportNetLogInfo.state == 'LOGGING') {
-        // Only possible to stop logging. Radio buttons reflects current state.
-        document
-            .querySelector(
-                'input[name="log-mode"][value="' +
-                exportNetLogInfo.captureMode + '"]')
-            .checked = true;
-        $('export-view-stop-data').disabled = false;
-      } else if (exportNetLogInfo.state == 'UNINITIALIZED') {
-        $('export-view-file-path-text').textContent =
-            'Unable to initialize NetLog data file.';
-      }
+    /**
+     * Updates the UI to display the "initial" state. This is the state when
+     * logging has not been started yet, and there are controls to start
+     * logging.
+     */
+    renderInitial_: function(info) {
+      this.showStateDiv_(kIdStateDivInitial);
+      $(kIdStartLoggingButton).onclick = this.onStartLogging_.bind(this);
+    },
+
+    /**
+     * Updates the UI to display the "logging" state. This is the state while
+     * capturing is in progress and being written to disk.
+     */
+    renderLogging_: function(info) {
+      this.showStateDiv_(kIdStateDivLogging);
+
+      $(kIdStopLoggingButton).onclick = this.onStopLogging_.bind(this);
+      $(kIdCaptureModeLogging).textContent = this.getCaptureModeText_(info);
+      $(kIdFilePathLogging).textContent = info.file;
     },
 
     /*
-     * Returns true if the UI is being displayed for mobile, otherwise false
-     * for desktop. This is controlled by the HTML template.
+     * Updates the UI to display the state when logging has stopped.
+     */
+    renderStoppedLogging_: function(info) {
+      this.showStateDiv_(kIdStateDivStopped);
+
+      // The email button is only available in the mobile UI.
+      if ($(kIdEmailLogButton))
+        $(kIdEmailLogButton).onclick = this.onSendEmail_.bind(this);
+      $(kIdStartOverButton).onclick = this.onStartOver_.bind(this);
+
+      $(kIdFilePathStoppedLogging).textContent = info.file;
+
+      $(kIdCaptureModeStopped).textContent = this.getCaptureModeText_(info);
+
+      // Hook up the "read more..." link for privacy information.
+      $(kIdReadMoreLink).onclick = this.showPrivacyReadMore_.bind(this, true);
+      this.showPrivacyReadMore_(false);
+    },
+
+    /**
+     * Get the textual label for a capture mode from the HTML.

[xunjieli, 2017/05/02 17:10:01]

extra nitpicky: s/Get/Gets

[eroman, 2017/05/02 20:07:58]

Done.

      */
-    useMobileUI_: function() {
-      return !!document.getElementById('export-view-mobile-send-data');
-    }
+    getCaptureModeText_: function(info) {
+      if (!info.logCaptureModeKnown)
+        return "Unknown";
+
+      let captureMode = info.captureMode;

[xunjieli, 2017/05/02 17:10:01]

nit: this local variable is used in exactly one place, could consider inlining
it?

[eroman, 2017/05/02 20:07:59]

Done.

+
+      var radioButton = document.querySelector(
+          'input[name="log-mode"][value="' + captureMode + '"]');
+      if (!radioButton)
+        return 'Unknown';
+      return radioButton.parentElement.textContent;
+    },
+
+    showPrivacyReadMore_: function(show) {
+      $(kIdReadMoreDiv).hidden = !show;
+      $(kIdReadMoreLink).hidden = show;
+    },
+
+    showStateDiv_: function(divId) {
+      var kAllDivIds = [
+        kIdStateDivUninitialized,
+        kIdStateDivInitial,
+        kIdStateDivLogging,
+        kIdStateDivStopped
+      ];
+
+      for (var curDivId of kAllDivIds) {
+        $(curDivId).hidden = divId != curDivId;
+      }
+    },
   };
 
   return NetExportView;