Fix sending multiple feedback reports within short durations of each other

chrome/browser/resources/feedback/js/feedback.js

 // Copyright 2013 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.
 
 /** @type {string}
  * @const
  */
 var FEEDBACK_LANDING_PAGE =
     'https://support.google.com/chrome/go/feedback_confirmation';
 
@@ skipping 45 matching lines @@
  * @enum {string}
  */
 var FeedbackFlow = {
   REGULAR: 'regular',  // Flow in a regular user session.
   LOGIN: 'login'       // Flow on the login screen.
 };
 
 var attachedFileBlob = null;
 var lastReader = null;
 
-var feedbackInfo = null;
+/**
+ * Determines whether the system information associated with this instance of
+ * the feedback window has been received.
+ * @type {boolean}
+ */
+var isSystemInfoReady = false;
 
 /**
  * The callback used by the sys_info_page to receive the event that the system
  * information is ready.
  * @type {function(sysInfo)}
  */
 var sysInfoPageOnSysInfoReadyCallback = null;
 
 /**
  * Reads the selected file when the user selects a file.
@@ skipping 92 matching lines @@
     feedbackInfo.traceId = null;
   }
 </if>
 
   feedbackInfo.sendHistograms = useHistograms;
 
   // If the user doesn't want to send the screenshot.
   if (!$('screenshot-checkbox').checked)
     feedbackInfo.screenshot = null;
 
-  if (useSystemInfo) {
-    // The user chose to include the system information as part of the report.
-    // If they are not ready yet, we have to send the report later.
-    if (!isSystemInfoReady()) {
-      // The report will be sent later by the feedback extension's background
-      // page (see event_handler.js).
-      sendReportLater(feedbackInfo);
+  // Clear the system information if the user doesn't want it to be sent.
+  if (!useSystemInfo)
+    feedbackInfo.systemInformation = null;

[xiyuan, 2016/03/16 16:39:28]

It is probably safer to remember useSystemInfo in FeedbackRequest.sendReport in
and do this in FeedbackRequest.sendReportNow. Just in case, we clear it too
early here while the getSystemInformation call is still in fly.

[afakhry, 2016/03/16 17:51:22]

Thanks for catching this. Done.

 
-      // No landing page for login screen feedback.
-      if (feedbackInfo.flow != FeedbackFlow.LOGIN)
-        window.open(FEEDBACK_LANDING_PAGE, '_blank');
-      window.close();
-      return true;
-    }
-  }
-
-  chrome.feedbackPrivate.sendFeedback(feedbackInfo, function(result) {
-    // No landing page for login screen feedback.
-    if (feedbackInfo.flow != FeedbackFlow.LOGIN)
-      window.open(FEEDBACK_LANDING_PAGE, '_blank');
-    window.close();
-  });
-
+  // Request sending the report, show the landing page (if allowed), and close
+  // this window right away. The FeedbackRequest object that represents this
+  // report will take care of sending the report in the background.
+  sendFeedbackReport(useSystemInfo);
+  if (feedbackInfo.flow != FeedbackFlow.LOGIN)
+    window.open(FEEDBACK_LANDING_PAGE, '_blank');
+  window.close();
   return true;
 }
 
 /**
  * Click listener for the cancel button.
  * @param {Event} e The click event being handled.
  */
 function cancel(e) {
   e.preventDefault();
   window.close();
@@ skipping 48 matching lines @@
   if (feedbackInfo.flow == FeedbackFlow.LOGIN)
     minHeight = FEEDBACK_MIN_HEIGHT_LOGIN;
   height = Math.max(height, minHeight);
 
   chrome.app.window.current().resizeTo(width, height);
 }
 
 /**
  * A callback to be invoked when the background page of this extension receives
  * the system information.
- * @param {Object} sysInfo The received system information.
  */
-function onSystemInformation(sysInfo) {
-  // Concatenate the feedbackInfo's systemInformation with the newly received
-  // sysInfo once (if any).
-  if (feedbackInfo.systemInformation) {
-    feedbackInfo.systemInformation =
-      feedbackInfo.systemInformation.concat(sysInfo);
-  } else {
-    feedbackInfo.systemInformation = sysInfo;
-  }
-
+function onSystemInformation() {
+  isSystemInfoReady = true;
   // In case the sys_info_page needs to be notified by this event, do so.
   if (sysInfoPageOnSysInfoReadyCallback != null) {
     sysInfoPageOnSysInfoReadyCallback(feedbackInfo.systemInformation);
     sysInfoPageOnSysInfoReadyCallback = null;
   }
 }
 
 /**
  * Initializes our page.
  * Flow:
  * .) DOMContent Loaded        -> . Request feedbackInfo object
  *                                . Setup page event handlers
  * .) Feedback Object Received -> . take screenshot
  *                                . request email
  *                                . request System info
  *                                . request i18n strings
  * .) Screenshot taken         -> . Show Feedback window.
  */
 function initialize() {
   // Add listener to receive the feedback info object.
   chrome.runtime.onMessage.addListener(function(request, sender, sendResponse) {
     if (request.sentFromEventPage) {
-      feedbackInfo = request.data;
-
       if (!feedbackInfo.flow)
         feedbackInfo.flow = FeedbackFlow.REGULAR;
 
       $('description-text').textContent = feedbackInfo.description;
       if (feedbackInfo.pageUrl)
         $('page-url-text').value = feedbackInfo.pageUrl;
 
       takeScreenshot(function(screenshotCanvas) {
         // We've taken our screenshot, show the feedback page without any
         // further delay.
         window.webkitRequestAnimationFrame(function() {
           resizeAppWindow();
         });
         chrome.app.window.current().show();
 
         var screenshotDataUrl = screenshotCanvas.toDataURL('image/png');
         $('screenshot-image').src = screenshotDataUrl;
         $('screenshot-image').classList.toggle('wide-screen',
             $('screenshot-image').width > MAX_SCREENSHOT_WIDTH);
         feedbackInfo.screenshot = dataUrlToBlob(screenshotDataUrl);
       });
 
       chrome.feedbackPrivate.getUserEmail(function(email) {
         $('user-email-text').value = email;
       });
 
-      // Initiate getting the system info and use it to prepare the full
-      // feedback info.
+      // Initiate getting the system info.
+      isSystemInfoReady = false;
       getSystemInformation(onSystemInformation);
 
       // An extension called us with an attached file.
       if (feedbackInfo.attachedFile) {
         $('attached-filename-text').textContent =
             feedbackInfo.attachedFile.name;
         attachedFileBlob = feedbackInfo.attachedFile.data;
         $('custom-file-container').hidden = false;
         $('attach-file').hidden = true;
       }
@@ skipping 34 matching lines @@
                 width: 600,
                 height: 400,
                 hidden: false,
                 resizable: true
               }, function(appWindow) {
                 // Define functions for the newly created window.
 
                 // Gets the full system information for the new window.
                 appWindow.contentWindow.getFullSystemInfo =
                     function(callback) {
-                      if (isSystemInfoReady()) {
+                      if (isSystemInfoReady) {
                         callback(feedbackInfo.systemInformation);
                         return;
                       }
 
                       sysInfoPageOnSysInfoReadyCallback = callback;
                     };
 
                 // Returns the loadTimeData for the new window.
                 appWindow.contentWindow.getLoadTimeData = function() {
                   return loadTimeData;
@@ skipping 22 matching lines @@
     $('cancel-button').onclick = cancel;
     $('remove-attached-file').onclick = clearAttachedFile;
 <if expr="chromeos">
     $('performance-info-checkbox').addEventListener(
         'change', performanceFeedbackChanged);
 </if>
   });
 }
 
 initialize();