Bug 1357297 - Restrict media decode issues that show user notification - r?jwwang draft
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 19 Apr 2017 09:42:45 +1200
changeset 564762 3f5c54539f31899e3d954eeff13d9cbe2729b50b
parent 564761 deaa3b874723ab07f798a169bdfce4c75ae9c4c9
child 624833 d2baf17b267d44e58f9e866f2a069038b3d418e6
push id54696
push usergsquelart@mozilla.com
push dateWed, 19 Apr 2017 02:46:16 +0000
reviewersjwwang
bugs1357297
milestone55.0a1
Bug 1357297 - Restrict media decode issues that show user notification - r?jwwang The media prefs "media.decoder-doctor.decode-{errors,warnings}-allowed" list which decode issue codes will show the "Report Site Issue" notification. Default: NS_ERROR_DOM_MEDIA_DEMUXER_ERR and NS_ERROR_DOM_MEDIA_METADATA_ERR, which are the kinds of errors we currently care most about, and have more chance to fix. MozReview-Commit-ID: JbY2pwv4TXP
dom/media/DecoderDoctorDiagnostics.cpp
modules/libpref/init/all.js
--- a/dom/media/DecoderDoctorDiagnostics.cpp
+++ b/dom/media/DecoderDoctorDiagnostics.cpp
@@ -405,16 +405,43 @@ AllowNotification(const NotificationAndR
   //                                  dom.properties) is one of them.
   // - Nothing (missing or empty) -> Disable everything.
   nsAdoptingCString filter =
     Preferences::GetCString("media.decoder-doctor.notifications-allowed");
   return filter.EqualsLiteral("*") ||
          StringListContains(filter, aNotification.mReportStringId);
 }
 
+static bool
+AllowDecodeIssue(const MediaResult& aDecodeIssue, bool aDecodeIssueIsError)
+{
+  if (aDecodeIssue == NS_OK) {
+    // 'NS_OK' means we are not actually reporting a decode issue, so we
+    // allow the report.
+    return true;
+  }
+
+  // "media.decoder-doctor.decode-{errors,warnings}-allowed" controls which
+  // decode issues may be dispatched to the front-end. It either contains:
+  // - '*' -> Allow everything.
+  // - Comma-separater list of ids -> Allow if the issue name is one of them.
+  // - Nothing (missing or empty) -> Disable everything.
+  nsAdoptingCString filter =
+    Preferences::GetCString(aDecodeIssueIsError
+                            ? "media.decoder-doctor.decode-errors-allowed"
+                            : "media.decoder-doctor.decode-warnings-allowed");
+  if (filter.EqualsLiteral("*")) {
+    return true;
+  }
+
+  nsCString decodeIssueName;
+  GetErrorName(aDecodeIssue.Code(), static_cast<nsACString&>(decodeIssueName));
+  return StringListContains(filter, decodeIssueName);
+}
+
 static void
 ReportAnalysis(nsIDocument* aDocument,
                const NotificationAndReportStringId& aNotification,
                bool aIsSolved,
                const nsAString& aFormats = NS_LITERAL_STRING(""),
                const MediaResult& aDecodeIssue = NS_OK,
                bool aDecodeIssueIsError = true,
                const nsACString& aDocURL = NS_LITERAL_CSTRING(""),
@@ -457,17 +484,18 @@ ReportAnalysis(nsIDocument* aDocument,
       default:
         MOZ_ASSERT_UNREACHABLE("Bad notification parameter choice");
         break;
       }
     }
     ReportToConsole(aDocument, aNotification.mReportStringId, params);
   }
 
-  if (AllowNotification(aNotification)) {
+  if (AllowNotification(aNotification) &&
+      AllowDecodeIssue(aDecodeIssue, aDecodeIssueIsError)) {
     DispatchNotification(
       aDocument->GetInnerWindow(), aNotification, aIsSolved,
       aFormats,
       decodeIssueDescription,
       aDocURL,
       aResourceURL);
   }
 }
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -385,16 +385,18 @@ pref("media.gmp.storage.version.expected
 
 // Filter what triggers user notifications.
 // See DecoderDoctorDocumentWatcher::ReportAnalysis for details.
 #ifdef NIGHTLY_BUILD
 pref("media.decoder-doctor.notifications-allowed", "MediaWMFNeeded,MediaWidevineNoWMF,MediaCannotInitializePulseAudio,MediaCannotPlayNoDecoders,MediaUnsupportedLibavcodec,MediaDecodeError");
 #else
 pref("media.decoder-doctor.notifications-allowed", "MediaWMFNeeded,MediaWidevineNoWMF,MediaCannotInitializePulseAudio,MediaCannotPlayNoDecoders,MediaUnsupportedLibavcodec");
 #endif
+pref("media.decoder-doctor.decode-errors-allowed", "NS_ERROR_DOM_MEDIA_DEMUXER_ERR, NS_ERROR_DOM_MEDIA_METADATA_ERR");
+pref("media.decoder-doctor.decode-warnings-allowed", "NS_ERROR_DOM_MEDIA_DEMUXER_ERR, NS_ERROR_DOM_MEDIA_METADATA_ERR");
 // Whether we report partial failures.
 pref("media.decoder-doctor.verbose", false);
 // Whether DD should consider WMF-disabled a WMF failure, useful for testing.
 pref("media.decoder-doctor.wmf-disabled-is-failure", false);
 // URL to report decode issues
 pref("media.decoder-doctor.new-issue-endpoint", "https://webcompat.com/issues/new");
 
 // Whether to suspend decoding of videos in background tabs.