Bug 1343442 - Analyze eDecodeError/Warning issues - r?jya draft
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 24 Mar 2017 17:09:41 +1100
changeset 560836 43c4c7bd3e48770bb1cdb5aa3e310e4420aa2540
parent 560835 6ba03a16ae03767aaee98116ce727446dc83d1fa
child 560837 4022c9d730c64697e40496f05a0c0a238fa923ba
push id53551
push usergsquelart@mozilla.com
push dateTue, 11 Apr 2017 23:47:44 +0000
reviewersjya
bugs1343442
milestone55.0a1
Bug 1343442 - Analyze eDecodeError/Warning issues - r?jya MozReview-Commit-ID: 97AhQz2Hgmi
dom/media/DecoderDoctorDiagnostics.cpp
--- a/dom/media/DecoderDoctorDiagnostics.cpp
+++ b/dom/media/DecoderDoctorDiagnostics.cpp
@@ -240,17 +240,20 @@ DecoderDoctorDocumentWatcher::EnsureTime
 }
 
 enum class ReportParam : uint8_t
 {
   // Marks the end of the parameter list.
   // Keep this zero! (For implicit zero-inits when used in definitions below.)
   None = 0,
 
-  Formats
+  Formats,
+  DecodeIssue,
+  DocURL,
+  ResourceURL
 };
 
 struct NotificationAndReportStringId
 {
   // Notification type, handled by browser-media.js.
   dom::DecoderDoctorNotificationType mNotificationType;
   // Console message id. Key in dom/locales/.../chrome/dom/dom.properties.
   const char* mReportStringId;
@@ -275,46 +278,68 @@ static const NotificationAndReportString
   { dom::DecoderDoctorNotificationType::Can_play_but_some_missing_decoders,
     "MediaNoDecoders", { ReportParam::Formats } };
 static const NotificationAndReportStringId sCannotInitializePulseAudio =
   { dom::DecoderDoctorNotificationType::Cannot_initialize_pulseaudio,
     "MediaCannotInitializePulseAudio", { ReportParam::None } };
 static const NotificationAndReportStringId sUnsupportedLibavcodec =
   { dom::DecoderDoctorNotificationType::Unsupported_libavcodec,
     "MediaUnsupportedLibavcodec", { ReportParam::None } };
+static const NotificationAndReportStringId sMediaDecodeError =
+  { dom::DecoderDoctorNotificationType::Decode_error,
+    "MediaDecodeError",
+    { ReportParam::ResourceURL, ReportParam::DecodeIssue } };
+static const NotificationAndReportStringId sMediaDecodeWarning =
+  { dom::DecoderDoctorNotificationType::Decode_warning,
+    "MediaDecodeWarning",
+    { ReportParam::ResourceURL, ReportParam::DecodeIssue } };
 
 static const NotificationAndReportStringId *const
 sAllNotificationsAndReportStringIds[] =
 {
   &sMediaWidevineNoWMF,
   &sMediaWMFNeeded,
   &sMediaPlatformDecoderNotFound,
   &sMediaCannotPlayNoDecoders,
   &sMediaNoDecoders,
   &sCannotInitializePulseAudio,
   &sUnsupportedLibavcodec,
+  &sMediaDecodeError,
+  &sMediaDecodeWarning
 };
 
 static void
 DispatchNotification(nsISupports* aSubject,
                      const NotificationAndReportStringId& aNotification,
                      bool aIsSolved,
-                     const nsAString& aFormats)
+                     const nsAString& aFormats,
+                     const nsAString& aDecodeIssue,
+                     const nsACString& aDocURL,
+                     const nsAString& aResourceURL)
 {
   if (!aSubject) {
     return;
   }
   dom::DecoderDoctorNotification data;
   data.mType = aNotification.mNotificationType;
   data.mIsSolved = aIsSolved;
   data.mDecoderDoctorReportId.Assign(
     NS_ConvertUTF8toUTF16(aNotification.mReportStringId));
   if (!aFormats.IsEmpty()) {
     data.mFormats.Construct(aFormats);
   }
+  if (!aDecodeIssue.IsEmpty()) {
+    data.mDecodeIssue.Construct(aDecodeIssue);
+  }
+  if (!aDocURL.IsEmpty()) {
+    data.mDocURL.Construct(NS_ConvertUTF8toUTF16(aDocURL));
+  }
+  if (!aResourceURL.IsEmpty()) {
+    data.mResourceURL.Construct(aResourceURL);
+  }
   nsAutoString json;
   data.ToJSON(json);
   if (json.IsEmpty()) {
     DD_WARN("DecoderDoctorDiagnostics/DispatchEvent() - Could not create json for dispatch");
     // No point in dispatching this notification without data, the front-end
     // wouldn't know what to display.
     return;
   }
@@ -329,37 +354,45 @@ static void
 ReportToConsole(nsIDocument* aDocument,
                 const char* aConsoleStringId,
                 nsTArray<const char16_t*>& aParams)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aDocument);
 
   DD_DEBUG("DecoderDoctorDiagnostics.cpp:ReportToConsole(doc=%p) ReportToConsole"
-           " - aMsg='%s' params={%s}",
+           " - aMsg='%s' params={%s%s%s%s}",
            aDocument, aConsoleStringId,
            aParams.IsEmpty()
            ? "<no params>"
-           : NS_ConvertUTF16toUTF8(aParams[0]).get());
+           : NS_ConvertUTF16toUTF8(aParams[0]).get(),
+           (aParams.Length() < 1 || !aParams[1]) ? "" : ", ",
+           (aParams.Length() < 1 || !aParams[1])
+           ? ""
+           : NS_ConvertUTF16toUTF8(aParams[1]).get(),
+           aParams.Length() < 2 ? "" : ", ...");
   nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
                                   NS_LITERAL_CSTRING("Media"),
                                   aDocument,
                                   nsContentUtils::eDOM_PROPERTIES,
                                   aConsoleStringId,
                                   aParams.IsEmpty()
                                   ? nullptr
                                   : aParams.Elements(),
                                   aParams.Length());
 }
 
 static void
 ReportAnalysis(nsIDocument* aDocument,
                const NotificationAndReportStringId& aNotification,
                bool aIsSolved,
-               const nsAString& aFormats = NS_LITERAL_STRING(""))
+               const nsAString& aFormats = NS_LITERAL_STRING(""),
+               const nsAString& aDecodeIssue = NS_LITERAL_STRING(""),
+               const nsACString& aDocURL = NS_LITERAL_CSTRING(""),
+               const nsAString& aResourceURL = NS_LITERAL_STRING(""))
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!aDocument) {
     return;
   }
 
   // Report non-solved issues to console.
@@ -370,16 +403,25 @@ ReportAnalysis(nsIDocument* aDocument,
     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());
+        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);
   }
 
@@ -390,17 +432,21 @@ ReportAnalysis(nsIDocument* aDocument,
   //                                  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)) {
     DispatchNotification(
-      aDocument->GetInnerWindow(), aNotification, aIsSolved, aFormats);
+      aDocument->GetInnerWindow(), aNotification, aIsSolved,
+      aFormats,
+      aDecodeIssue,
+      aDocURL,
+      aResourceURL);
   }
 }
 
 static nsString
 CleanItemForFormatsList(const nsAString& aItem)
 {
   nsString item(aItem);
   // Remove commas from item, as commas are used to separate items. It's fine
@@ -421,16 +467,31 @@ 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:
@@ -439,16 +500,22 @@ DecoderDoctorDocumentWatcher::Synthesize
 #endif
 #if defined(MOZ_FFMPEG)
   nsAutoString formatsRequiringFFMpeg;
 #endif
   nsAutoString supportedKeySystems;
   nsAutoString unsupportedKeySystems;
   DecoderDoctorDiagnostics::KeySystemIssue lastKeySystemIssue =
     DecoderDoctorDiagnostics::eUnset;
+  // Only deal with one decode error per document (the first one found).
+  const MediaResult* firstDecodeError = nullptr;
+  const nsString* firstDecodeErrorMediaSrc = nullptr;
+  // Only deal with one decode warning per document (the first one found).
+  const MediaResult* firstDecodeWarning = nullptr;
+  const nsString* firstDecodeWarningMediaSrc = nullptr;
 
   for (const auto& diag : mDiagnosticsSequence) {
     switch (diag.mDecoderDoctorDiagnostics.Type()) {
       case DecoderDoctorDiagnostics::eFormatSupportCheck:
         if (diag.mDecoderDoctorDiagnostics.CanPlay()) {
           AppendToFormatsList(playableFormats,
                               diag.mDecoderDoctorDiagnostics.Format());
         } else {
@@ -481,20 +548,28 @@ DecoderDoctorDocumentWatcher::Synthesize
             lastKeySystemIssue = issue;
           }
         }
         break;
       case DecoderDoctorDiagnostics::eEvent:
         MOZ_ASSERT_UNREACHABLE("Events shouldn't be stored for processing.");
         break;
       case DecoderDoctorDiagnostics::eDecodeError:
-        // TODO
+        if (!firstDecodeError) {
+          firstDecodeError = &diag.mDecoderDoctorDiagnostics.DecodeIssue();
+          firstDecodeErrorMediaSrc =
+            &diag.mDecoderDoctorDiagnostics.DecodeIssueMediaSrc();
+        }
         break;
       case DecoderDoctorDiagnostics::eDecodeWarning:
-        // TODO
+        if (!firstDecodeWarning) {
+          firstDecodeWarning = &diag.mDecoderDoctorDiagnostics.DecodeIssue();
+          firstDecodeWarningMediaSrc =
+            &diag.mDecoderDoctorDiagnostics.DecodeIssueMediaSrc();
+        }
         break;
       default:
         MOZ_ASSERT_UNREACHABLE("Unhandled DecoderDoctorDiagnostics type");
         break;
     }
   }
 
   // Check if issues have been solved, by finding if some now-playable
@@ -619,16 +694,39 @@ DecoderDoctorDocumentWatcher::Synthesize
 
     DD_INFO("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Can play media, but no decoders for some requested formats: %s",
             this, mDocument, NS_ConvertUTF16toUTF8(unplayableFormats).get());
     if (Preferences::GetBool("media.decoder-doctor.verbose", false)) {
       ReportAnalysis(mDocument, sMediaNoDecoders, false, unplayableFormats);
     }
     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),
+                   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),
+                   mDocument->GetDocumentURI()->GetSpecOrDefault(),
+                   *firstDecodeWarningMediaSrc);
+    return;
+  }
+
   DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Can play media, decoders available for all requested formats",
            this, mDocument);
 }
 
 void
 DecoderDoctorDocumentWatcher::AddDiagnostics(DecoderDoctorDiagnostics&& aDiagnostics,
                                              const char* aCallSite)
 {