Bug 1357297 - Restrict media decode issues that show user notification - r=jwwang
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 19 Apr 2017 09:42:45 +1200
changeset 353883 967c7416140f1f58b641276af96d3c4dbc892dfb
parent 353882 d6a68652c1d888e4b7d2d269ef65f55a7212cfb5
child 353884 54e64f30caeaaad20180282943d7809a8c7bbde8
push id89374
push userkwierso@gmail.com
push dateThu, 20 Apr 2017 00:15:40 +0000
treeherdermozilla-inbound@ccfe5420876a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1357297
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.