Bug 1357297 - Refactor DecDoctor's ReportAnalysis - r?jwwang draft
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 19 Apr 2017 09:42:15 +1200
changeset 564761 deaa3b874723ab07f798a169bdfce4c75ae9c4c9
parent 564706 3f9f6d6086b2d247831d1d03d530095bebd5a6b2
child 564762 3f5c54539f31899e3d954eeff13d9cbe2729b50b
push id54696
push usergsquelart@mozilla.com
push dateWed, 19 Apr 2017 02:46:16 +0000
reviewersjwwang
bugs1357297
milestone55.0a1
Bug 1357297 - Refactor DecDoctor's ReportAnalysis - r?jwwang ReportAnalysis now takes the decode issue as a MediaResult and an error/warning flag instead of a string; And the notification-allowed test is in a separate function. These changes will simplify the next patch, where we introduce a new filter based on the decode issue. MozReview-Commit-ID: 3ZbtGEl427L
dom/media/DecoderDoctorDiagnostics.cpp
--- a/dom/media/DecoderDoctorDiagnostics.cpp
+++ b/dom/media/DecoderDoctorDiagnostics.cpp
@@ -301,16 +301,31 @@ sAllNotificationsAndReportStringIds[] =
   &sMediaCannotPlayNoDecoders,
   &sMediaNoDecoders,
   &sCannotInitializePulseAudio,
   &sUnsupportedLibavcodec,
   &sMediaDecodeError,
   &sMediaDecodeWarning
 };
 
+// Create a webcompat-friendly description of a MediaResult.
+static nsString
+MediaResultDescription(const MediaResult& aResult, bool aIsError)
+{
+  nsCString name;
+  GetErrorName(aResult.Code(), name);
+  return NS_ConvertUTF8toUTF16(
+           nsPrintfCString(
+             "%s Code: %s (0x%08" PRIx32 ")%s%s",
+             aIsError ? "Error" : "Warning", name.get(),
+             static_cast<uint32_t>(aResult.Code()),
+             aResult.Message().IsEmpty() ? "" : "\nDetails: ",
+             aResult.Message().get()));
+}
+
 static void
 DispatchNotification(nsISupports* aSubject,
                      const NotificationAndReportStringId& aNotification,
                      bool aIsSolved,
                      const nsAString& aFormats,
                      const nsAString& aDecodeIssue,
                      const nsACString& aDocURL,
                      const nsAString& aResourceURL)
@@ -375,76 +390,88 @@ ReportToConsole(nsIDocument* aDocument,
                                   nsContentUtils::eDOM_PROPERTIES,
                                   aConsoleStringId,
                                   aParams.IsEmpty()
                                   ? nullptr
                                   : aParams.Elements(),
                                   aParams.Length());
 }
 
+static bool
+AllowNotification(const NotificationAndReportStringId& aNotification)
+{
+  // "media.decoder-doctor.notifications-allowed" controls which notifications
+  // may be dispatched to the front-end. It either contains:
+  // - '*' -> Allow everything.
+  // - Comma-separater list of ids -> Allow if aReportStringId (from
+  //                                  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 void
 ReportAnalysis(nsIDocument* aDocument,
                const NotificationAndReportStringId& aNotification,
                bool aIsSolved,
                const nsAString& aFormats = NS_LITERAL_STRING(""),
-               const nsAString& aDecodeIssue = NS_LITERAL_STRING(""),
+               const MediaResult& aDecodeIssue = NS_OK,
+               bool aDecodeIssueIsError = true,
                const nsACString& aDocURL = NS_LITERAL_CSTRING(""),
                const nsAString& aResourceURL = NS_LITERAL_STRING(""))
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!aDocument) {
     return;
   }
 
+  nsString decodeIssueDescription;
+  if (aDecodeIssue != NS_OK) {
+    decodeIssueDescription.Assign(MediaResultDescription(aDecodeIssue,
+                                                         aDecodeIssueIsError));
+  }
+
   // Report non-solved issues to console.
   if (!aIsSolved) {
     // Build parameter array needed by console message.
     AutoTArray<const char16_t*,
                NotificationAndReportStringId::maxReportParams> params;
     for (int i = 0; i < NotificationAndReportStringId::maxReportParams; ++i) {
       if (aNotification.mReportParams[i] == ReportParam::None) {
         break;
       }
       switch (aNotification.mReportParams[i]) {
       case ReportParam::Formats:
         params.AppendElement(aFormats.Data());
         break;
       case ReportParam::DecodeIssue:
-        params.AppendElement(aDecodeIssue.Data());
+        params.AppendElement(decodeIssueDescription.Data());
         break;
       case ReportParam::DocURL:
         params.AppendElement(NS_ConvertUTF8toUTF16(aDocURL).Data());
         break;
       case ReportParam::ResourceURL:
         params.AppendElement(aResourceURL.Data());
         break;
       default:
         MOZ_ASSERT_UNREACHABLE("Bad notification parameter choice");
         break;
       }
     }
     ReportToConsole(aDocument, aNotification.mReportStringId, params);
   }
 
-  // "media.decoder-doctor.notifications-allowed" controls which notifications
-  // may be dispatched to the front-end. It either contains:
-  // - '*' -> Allow everything.
-  // - Comma-separater list of ids -> Allow if aReportStringId (from
-  //                                  dom.properties) is one of them.
-  // - Nothing (missing or empty) -> Disable everything.
-  nsAdoptingCString filter =
-    Preferences::GetCString("media.decoder-doctor.notifications-allowed");
-  filter.StripWhitespace();
-  if (filter.EqualsLiteral("*")
-      || StringListContains(filter, aNotification.mReportStringId)) {
+  if (AllowNotification(aNotification)) {
     DispatchNotification(
       aDocument->GetInnerWindow(), aNotification, aIsSolved,
       aFormats,
-      aDecodeIssue,
+      decodeIssueDescription,
       aDocURL,
       aResourceURL);
   }
 }
 
 static nsString
 CleanItemForFormatsList(const nsAString& aItem)
 {
@@ -467,31 +494,16 @@ AppendToFormatsList(nsAString& aList, co
 }
 
 static bool
 FormatsListContains(const nsAString& aList, const nsAString& aItem)
 {
   return StringListContains(aList, CleanItemForFormatsList(aItem));
 }
 
-// Create a webcompat-friendly description of a MediaResult.
-static nsString
-MediaResultDescription(const MediaResult& aResult, bool aIsError)
-{
-  nsCString name;
-  GetErrorName(aResult.Code(), static_cast<nsACString&>(name));
-  return NS_ConvertUTF8toUTF16(
-           nsPrintfCString(
-             "%s Code: %s (0x%08" PRIx32 ")%s%s",
-             aIsError ? "Error" : "Warning", name.get(),
-             static_cast<uint32_t>(aResult.Code()),
-             aResult.Message().IsEmpty() ? "" : "\nDetails: ",
-             aResult.Message().get()));
-}
-
 void
 DecoderDoctorDocumentWatcher::SynthesizeAnalysis()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   nsAutoString playableFormats;
   nsAutoString unplayableFormats;
   // Subsets of unplayableFormats that require a specific platform decoder:
@@ -700,28 +712,30 @@ DecoderDoctorDocumentWatcher::Synthesize
     return;
   }
 
   if (firstDecodeError) {
     DD_INFO("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Decode error: %s",
             this, mDocument, firstDecodeError->Description().get());
     ReportAnalysis(mDocument, sMediaDecodeError, false,
                    NS_LITERAL_STRING(""),
-                   MediaResultDescription(*firstDecodeError, true),
+                   *firstDecodeError,
+                   true, // aDecodeIssueIsError=true
                    mDocument->GetDocumentURI()->GetSpecOrDefault(),
                    *firstDecodeErrorMediaSrc);
     return;
   }
 
   if (firstDecodeWarning) {
     DD_INFO("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Decode warning: %s",
             this, mDocument, firstDecodeWarning->Description().get());
     ReportAnalysis(mDocument, sMediaDecodeWarning, false,
                    NS_LITERAL_STRING(""),
-                   MediaResultDescription(*firstDecodeWarning, false),
+                   *firstDecodeWarning,
+                   false, // aDecodeIssueIsError=false
                    mDocument->GetDocumentURI()->GetSpecOrDefault(),
                    *firstDecodeWarningMediaSrc);
     return;
   }
 
   DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Can play media, decoders available for all requested formats",
            this, mDocument);
 }