Completed functionality of Cast feedback dialog.

- Adds a default-disabled feature kCastFeedbackDialog to show the Cast feedback WebU - Launches new WebUI from Cast toolbar icon menu based on feature - Hooks up WebUI to chrome.feedbackPrivate to actually submit feedback Bug: 1173633 Change-Id: I74bb9606800e13eb42f46e1f807bc07c50a4b840 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2856709 Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Reviewed-by: Takumi Fujimoto <takumif@chromium.org> Reviewed-by: mark a. foltz <mfoltz@chromium.org> Commit-Queue: John Williams <jrw@chromium.org> Cr-Commit-Position: refs/heads/master@{#881106}
chromium · May 10, 2021 · 024e8d8 · 024e8d8
1 parent d79e023
commit 024e8d8
Show file tree

Hide file tree

Showing 14 changed files with 257 additions and 56 deletions.
diff --git a/chrome/app/media_router_strings.grdp b/chrome/app/media_router_strings.grdp
@@ -175,9 +175,6 @@ video files, instead of all files.">
   <message name="IDS_MEDIA_ROUTER_FEEDBACK_HEADER" desc="Header of the Media Router feedback page.">
   Tell us what's happening with Google Cast.
   </message>
-  <message name="IDS_MEDIA_ROUTER_FEEDBACK_OK" desc="Text for 'OK' button.">
-    OK
-  </message>
   <message name="IDS_MEDIA_ROUTER_FEEDBACK_YOUR_ANSWER" desc="Placeholder text for inputs in the feedback form.">
     Your answer
   </message>
@@ -211,12 +208,12 @@ video files, instead of all files.">
   <message name="IDS_MEDIA_ROUTER_FEEDBACK_EMAIL_FIELD" desc="Text for the email field in the feedback form.">
     Email (optional):
   </message>
-  <message name="IDS_MEDIA_ROUTER_FEEDBACK_CANCEL_BUTTON" desc="Text for the cancel button.">
-    Cancel
-  </message>
   <message name="IDS_MEDIA_ROUTER_FEEDBACK_SEND_BUTTON" desc="Text for the send feedback button.">
     Send Feedback
   </message>
+  <message name="IDS_MEDIA_ROUTER_FEEDBACK_DISCARD_CONFIRMATION" desc="Text in the confirmation that the user does not want to submit feedback.">
+    Do you want to discard the feedback?
+  </message>
 
   <!-- Cast feedback dialog: feedback types -->
   <message name="IDS_MEDIA_ROUTER_FEEDBACK_TYPE_BUG_OR_ERROR" desc="The label of the feedback type for Bug or Error.">
@@ -287,9 +284,6 @@ video files, instead of all files.">
   </message>
 
   <!-- Cast feedback dialog: steps of sending feedback -->
-  <message name="IDS_MEDIA_ROUTER_FEEDBACK_DISCARD_CONFIRMATION" desc="Text in the confirmation that the user does not want to submitfeedback.">
-    Do you want to discard the feedback?
-  </message>
   <message name="IDS_MEDIA_ROUTER_FEEDBACK_SENDING" desc="Shown while we're attempting to send feedback.">
     Sending feedback...
   </message>

diff --git a/chrome/app/media_router_strings_grdp/IDS_MEDIA_ROUTER_FEEDBACK_CANCEL_BUTTON.png.sha1 b/chrome/app/media_router_strings_grdp/IDS_MEDIA_ROUTER_FEEDBACK_CANCEL_BUTTON.png.sha1
diff --git a/chrome/app/media_router_strings_grdp/IDS_MEDIA_ROUTER_FEEDBACK_OK.png.sha1 b/chrome/app/media_router_strings_grdp/IDS_MEDIA_ROUTER_FEEDBACK_OK.png.sha1
diff --git a/chrome/browser/media/router/media_router_feature.cc b/chrome/browser/media/router/media_router_feature.cc
@@ -48,6 +48,8 @@ const base::Feature kAllowAllSitesToInitiateMirroring{
     "AllowAllSitesToInitiateMirroring", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kCastToMeetingFromCastDialog{
     "CastToMeetingFromCastDialog", base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kCastFeedbackDialog{"CastFeedbackDialog",
+                                        base::FEATURE_DISABLED_BY_DEFAULT};
 #endif  // !defined(OS_ANDROID)
 
 #if defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)

diff --git a/chrome/browser/media/router/media_router_feature.h b/chrome/browser/media/router/media_router_feature.h
@@ -48,6 +48,10 @@ extern const base::Feature kAllowAllSitesToInitiateMirroring;
 // If enabled, meetings appear as receivers in the Cast menu.
 extern const base::Feature kCastToMeetingFromCastDialog;
 
+// If enabled, the WebUI Cast feedback dialog is used instead of using the
+// version in the Media Router component extension.
+extern const base::Feature kCastFeedbackDialog;
+
 namespace prefs {
 // Pref name for the enterprise policy for allowing Cast devices on all IPs.
 constexpr char kMediaRouterCastAllowAllIPs[] =

diff --git a/chrome/browser/resources/media_router/BUILD.gn b/chrome/browser/resources/media_router/BUILD.gn
@@ -68,6 +68,7 @@ js_library("feedback_ui") {
     "//ui/webui/resources/cr_elements/cr_radio_group:cr_radio_group.m",
     "//ui/webui/resources/js:load_time_data.m",
   ]
+  externs_list = [ "$externs_path/feedback_private.js" ]
 }
 
 js_library("media_router_internals") {

diff --git a/chrome/browser/resources/media_router/feedback_ui.html b/chrome/browser/resources/media_router/feedback_ui.html
@@ -170,7 +170,7 @@
       </div>
       <textarea placeholder="$i18n{yourAnswer}"
                 rows="8" cols="60"
-                value="{{feedbackDescription_::input}}">
+                value="{{comments_::input}}">
       </textarea>
     </div>
     <template is="dom-if"
@@ -263,9 +263,10 @@
           <span class="required-message"
                 hidden="[[sufficientFeedback_]]">*</span>
         </div>
-        <cr-input placeholder="$i18n{yourAnswer}"
-                  value="{{comments_}}">
-        </cr-input>
+        <textarea placeholder="$i18n{yourAnswer}"
+                  rows="8" cols="60"
+                  value="{{comments_::input}}">
+        </textarea>
       </div>
     </template>
     <template is="dom-if"
@@ -322,8 +323,10 @@
           <span class="required-message"
                 hidden="[[sufficientFeedback_]]">*</span>
         </div>
-        <cr-input type="text" value="{{comments_}}"
-                  placeholder="$i18n{yourAnswer}"></cr-input>
+        <textarea placeholder="$i18n{yourAnswer}"
+                  rows="8" cols="60"
+                  value="{{comments_::input}}">
+        </textarea>
       </div>
     </template>
     <div class="question">
@@ -345,7 +348,7 @@
     <div id="form-buttons">
       <cr-button class="cancel-button"
                  on-click="onCancel_">
-        $i18n{cancelButton}
+        $i18n{cancel}
       </cr-button>
       <cr-button class="action-button"
                  on-click="onSubmit_"
@@ -355,6 +358,18 @@
     </div>
   </div>
 
+  <cr-dialog id="sendDialog">
+    <div slot="body">
+      [[sendDialogText_]]
+    </div>
+    <div slot="button-container">
+      <cr-button on-click="onSendDialogOk_"
+                 hidden="[[!sendDialogIsInteractive_]]">
+        $i18n{ok}
+      </cr-button>
+    </div>
+  </cr-dialog>
+
   <cr-dialog id="logsDialog">
     <div slot="title">$i18n{logsHeader}</div>
     <div slot="body">

diff --git a/chrome/browser/resources/media_router/feedback_ui.js b/chrome/browser/resources/media_router/feedback_ui.js
@@ -15,7 +15,6 @@ import '//resources/cr_elements/shared_vars_css.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-
 /** @enum {string} */
 const FeedbackType = {
   BUG: 'Bug',
@@ -25,7 +24,36 @@ const FeedbackType = {
   OTHER: 'Other'
 };
 
+/**
+ * Keep in sync with UMA MediaRouterCastFeedbackEvent enum.
+ * @enum {number}
+ */
+const FeedbackEvent = {
+  OPENED: 0,
+  SENDING: 1,
+  RESENDING: 2,
+  SUCCEEDED: 3,
+  FAILED: 4,
+  MAX_VALUE: 4,
+};
+
+const MAX_SEND_ATTEMPTS = 4;
+const RESEND_DELAY_MS = 10000;
+
 export class FeedbackUiElement extends PolymerElement {
+  constructor() {
+    super();
+
+    /** @private {boolean} */
+    this.feedbackSent_ = false;
+
+    chrome.feedbackPrivate.getUserEmail(email => {
+      this.userEmail_ = email;
+    });
+
+    this.recordEvent_(FeedbackEvent.OPENED);
+  }
+
   static get is() {
     return 'feedback-ui';
   }
@@ -44,9 +72,6 @@ export class FeedbackUiElement extends PolymerElement {
       /** @private */
       comments_: String,
 
-      /** @private */
-      feedbackDescription_: String,
-
       /**
        * Possible values of |feedbackType_| for use in HTML.
        * @private @const {!Object<string, string>}
@@ -66,7 +91,10 @@ export class FeedbackUiElement extends PolymerElement {
       },
 
       /** @private */
-      hasNetworkSoftware_: Boolean,
+      hasNetworkSoftware_: String,
+
+      /** @private */
+      networkDescription_: String,
 
       /** @private */
       logData_: {
@@ -79,14 +107,22 @@ export class FeedbackUiElement extends PolymerElement {
       /** @private */
       projectedContentUrl_: String,
 
+      /** @private */
+      sendDialogText_: String,
+
+      /** @private */
+      sendDialogIsInteractive_: Boolean,
+
       /**
        * Set by onFeedbackChanged_() to control whether the "submit" button is
        * active.
        * @private
        */
       sufficientFeedback_: {
         type: Boolean,
-        value: false,
+        computed:
+            'computeSufficientFeedback_(feedbackType_, videoSmoothness_, ' +
+            'videoQuality_, audioQuality_, comments_, visibleInSetup_)',
       },
 
       /** @private */
@@ -99,18 +135,10 @@ export class FeedbackUiElement extends PolymerElement {
       videoSmoothness_: String,
 
       /** @private */
-      visibleInSetup_: Boolean,
+      visibleInSetup_: String,
     };
   }
 
-  static get observers() {
-    return [
-      'onFeedbackChanged_(feedbackType_, videoSmoothness_, ' +
-          'videoQuality_, audioQuality_, feedbackDescription_, ' +
-          'comments_, visibleInSetup_)',
-    ];
-  }
-
   /** @override */
   ready() {
     super.ready();
@@ -122,19 +150,16 @@ export class FeedbackUiElement extends PolymerElement {
   }
 
   /** @private */
-  onFeedbackChanged_() {
+  computeSufficientFeedback_() {
     switch (this.feedbackType_) {
       case FeedbackType.MIRRORING_QUALITY:
-        this.sufficientFeedback_ = Boolean(
+        return Boolean(
             this.videoSmoothness_ || this.videoQuality_ || this.audioQuality_ ||
             this.comments_);
-        break;
       case FeedbackType.DISCOVERY:
-        this.sufficientFeedback_ =
-            Boolean(this.visibleInSetup_ || this.comments_);
-        break;
+        return Boolean(this.visibleInSetup_ || this.comments_);
       default:
-        this.sufficientFeedback_ = Boolean(this.feedbackDescription_);
+        return Boolean(this.comments_);
     }
   }
 
@@ -170,14 +195,121 @@ export class FeedbackUiElement extends PolymerElement {
 
   /** @private */
   onSubmit_() {
-    // TODO(jrw): Submit feedback data.
-    console.log('onSubmit_');
+    const parts = [`Type: ${this.feedbackType_}`, ''];
+    const append = (label, value) => {
+      if (value) {
+        parts.push(`${label}: ${value}`);
+      }
+    };
+
+    switch (this.feedbackType_) {
+      case FeedbackType.MIRRORING_QUALITY:
+        append('Video Smoothness', this.videoSmoothness_);
+        append('Video Quality', this.videoQuality_);
+        append('Audio', this.audioQuality_);
+        append('Projected Content/URL', this.projectedContentUrl_);
+        append('Comments', this.comments_);
+        break;
+      case FeedbackType.DISCOVERY:
+        append('Chromecast Visible in Setup', this.visibleInSetup_);
+        append(
+            'Using VPN/proxy/firewall/NAS Software', this.hasNetworkSoftware_);
+        append('Network Description', this.networkDescription_);
+        append('Comments', this.comments_);
+        break;
+      default:
+        parts.push(this.comments_);
+        break;
+    }
+
+    const feedback = {
+      productId: 85561,
+      description: parts.join('\n'),
+      email: this.userEmail_,
+      flow: chrome.feedbackPrivate.FeedbackFlow.REGULAR,
+      categoryTag: 'dev',
+    };
+    if (this.attachLogs_) {
+      feedback.attachedFile = {
+        name: 'log.json',
+        data: new Blob([this.logData_]),
+      };
+    }
+
+    this.updateSendDialog_(FeedbackEvent.SENDING, 'sending', false);
+    this.$.sendDialog.showModal();
+    this.trySendFeedback_(feedback, 0, 0);
+  }
+
+  /**
+   * Schedules an attempt to send feedback after |delayMs| milliseconds.
+   * @param {!chrome.feedbackPrivate.FeedbackInfo} feedback
+   * @param {number} failureCount
+   * @param {number} delayMs
+   * @private
+   */
+  trySendFeedback_(feedback, failureCount, delayMs) {
+    setTimeout(() => {
+      const sendStartTime = Date.now();
+      chrome.feedbackPrivate.sendFeedback(
+          feedback, (status, landingPageType) => {
+            if (status == chrome.feedbackPrivate.Status.SUCCESS) {
+              this.feedbackSent_ = true;
+              this.updateSendDialog_(
+                  FeedbackEvent.SUCCEEDED, 'sendSuccess', true);
+            } else if (failureCount < MAX_SEND_ATTEMPTS) {
+              this.updateSendDialog_(
+                  FeedbackEvent.RESENDING, 'resending', false);
+              const sendDuration = Date.now() - sendStartTime;
+              this.trySendFeedback_(
+                  feedback, failureCount + 1,
+                  Math.max(0, RESEND_DELAY_MS - sendDuration));
+            } else {
+              this.updateSendDialog_(FeedbackEvent.FAILED, 'sendFail', true);
+            }
+          });
+    }, delayMs);
+  }
+
+  /**
+   * Records an event using UMA.
+   * @param {FeedbackEvent} event
+   * @private
+   */
+  recordEvent_(event) {
+    chrome.send(
+        'metricsHandler:recordInHistogram',
+        ['MediaRouter.Cast.Feedback.Event', event, FeedbackEvent.MAX_VALUE]);
+  }
+
+  /**
+   * Updates the status of the "send" dialog and records the event.
+   * @param {FeedbackEvent} event
+   * @param {string} stringKey
+   * @param {boolean} isInteractive
+   * @private
+   */
+  updateSendDialog_(event, stringKey, isInteractive) {
+    this.recordEvent_(event);
+    this.sendDialogText_ = loadTimeData.getString(stringKey);
+    this.sendDialogIsInteractive_ = isInteractive;
+  }
+
+  /** @private */
+  onSendDialogOk_() {
+    if (this.feedbackSent_) {
+      chrome.send('close');
+    } else {
+      this.$.sendDialog.close();
+    }
   }
 
   /** @private */
   onCancel_() {
-    // TODO(jrw): Cancel in-progress submission of feedback data.
-    console.log('onCancel_');
+    if (!this.comments_ ||
+        confirm(loadTimeData.getString('discardConfirmation'))) {
+      chrome.send('close');
+    }
   }
 
   /** @private */