Bug 1271483 - p14. Demagicify ReportStringId strings - r=cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 25 May 2016 01:03:21 +1000
changeset 380487 64ced9bb675e97ff076a277709bdca5ca7aff4d9
parent 380486 43024a985814d5fc7c08bd2561b0ee16352fc604
child 380488 80f0b3cbff9e595922116fa384133c5d2c697860
push id21230
push usergsquelart@mozilla.com
push dateWed, 22 Jun 2016 01:46:41 +0000
reviewerscpearce
bugs1271483
milestone50.0a1
Bug 1271483 - p14. Demagicify ReportStringId strings - r=cpearce Combine notification type and web-console string id into structs, simpler to pass around, and will be useful to go through them when checking for solved issues. MozReview-Commit-ID: Hy3bMG3m12V
dom/media/DecoderDoctorDiagnostics.cpp
--- a/dom/media/DecoderDoctorDiagnostics.cpp
+++ b/dom/media/DecoderDoctorDiagnostics.cpp
@@ -21,16 +21,22 @@
 static mozilla::LazyLogModule sDecoderDoctorLog("DecoderDoctor");
 #define DD_LOG(level, arg, ...) MOZ_LOG(sDecoderDoctorLog, level, (arg, ##__VA_ARGS__))
 #define DD_DEBUG(arg, ...) DD_LOG(mozilla::LogLevel::Debug, arg, ##__VA_ARGS__)
 #define DD_INFO(arg, ...) DD_LOG(mozilla::LogLevel::Info, arg, ##__VA_ARGS__)
 #define DD_WARN(arg, ...) DD_LOG(mozilla::LogLevel::Warning, arg, ##__VA_ARGS__)
 
 namespace mozilla {
 
+struct NotificationAndReportStringId
+{
+  dom::DecoderDoctorNotificationType mNotificationType;
+  const char* mReportStringId;
+};
+
 // Class that collects a sequence of diagnostics from the same document over a
 // small period of time, in order to provide a synthesized analysis.
 //
 // Referenced by the document through a nsINode property, mTimer, and
 // inter-task captures.
 // When notified that the document is dead, or when the timer expires but
 // nothing new happened, StopWatching() will remove the document property and
 // timer (if present), so no more work will happen and the watcher will be
@@ -62,19 +68,18 @@ private:
   static void DestroyPropertyCallback(void* aObject,
                                       nsIAtom* aPropertyName,
                                       void* aPropertyValue,
                                       void* aData);
 
   static const uint32_t sAnalysisPeriod_ms = 1000;
   void EnsureTimerIsStarted();
 
-  void ReportAnalysis(dom::DecoderDoctorNotificationType aNotificationType,
+  void ReportAnalysis(const NotificationAndReportStringId& aNotification,
                       bool aIsSolved,
-                      const char* aReportStringId,
                       const nsAString& aFormats);
 
   void SynthesizeAnalysis();
 
   // Raw pointer to an nsIDocument.
   // Must be non-null during construction.
   // Nulled when we want to stop watching, because either:
   // 1. The document has been destroyed (notified through
@@ -345,31 +350,46 @@ StringListContains(const ListString& aLi
   for (const auto& listItem : MakeStringListRange(aList)) {
     if (listItem.Equals(aItem)) {
       return true;
     }
   }
   return false;
 }
 
+static const NotificationAndReportStringId sMediaWidevineNoWMFNoSilverlight =
+  { dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
+    "MediaWidevineNoWMFNoSilverlight" };
+static const NotificationAndReportStringId sMediaWMFNeeded =
+  { dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
+    "MediaWMFNeeded" };
+static const NotificationAndReportStringId sMediaPlatformDecoderNotFound =
+  { dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
+    "MediaPlatformDecoderNotFound" };
+static const NotificationAndReportStringId sMediaCannotPlayNoDecoders =
+  { dom::DecoderDoctorNotificationType::Cannot_play,
+    "MediaCannotPlayNoDecoders" };
+static const NotificationAndReportStringId sMediaNoDecoders =
+  { dom::DecoderDoctorNotificationType::Can_play_but_some_missing_decoders,
+    "MediaNoDecoders" };
+
 static void
 DispatchNotification(nsISupports* aSubject,
-                     dom::DecoderDoctorNotificationType aNotificationType,
+                     const NotificationAndReportStringId& aNotification,
                      bool aIsSolved,
-                     const char* aReportStringId,
                      const nsAString& aFormats)
 {
   if (!aSubject) {
     return;
   }
   dom::DecoderDoctorNotification data;
-  data.mType = aNotificationType;
+  data.mType = aNotification.mNotificationType;
   data.mIsSolved = aIsSolved;
   data.mDecoderDoctorReportId.Assign(
-    NS_ConvertUTF8toUTF16(aReportStringId));
+    NS_ConvertUTF8toUTF16(aNotification.mReportStringId));
   if (!aFormats.IsEmpty()) {
     data.mFormats.Construct(aFormats);
   }
   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
@@ -380,57 +400,55 @@ DispatchNotification(nsISupports* aSubje
   nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
   if (obs) {
     obs->NotifyObservers(aSubject, "decoder-doctor-notification", json.get());
   }
 }
 
 void
 DecoderDoctorDocumentWatcher::ReportAnalysis(
-  dom::DecoderDoctorNotificationType aNotificationType,
+  const NotificationAndReportStringId& aNotification,
   bool aIsSolved,
-  const char* aReportStringId,
   const nsAString& aParams)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mDocument) {
     return;
   }
 
   // Report non-solved issues to console.
   if (!aIsSolved) {
     // 'params' will only be forwarded for non-empty strings.
     const char16_t* params[1] = { aParams.Data() };
     DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::ReportAnalysis() ReportToConsole - aMsg='%s' params[0]='%s'",
-             this, mDocument, aReportStringId,
+             this, mDocument, aNotification.mReportStringId,
              aParams.IsEmpty() ? "<no params>" : NS_ConvertUTF16toUTF8(params[0]).get());
     nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
                                     NS_LITERAL_CSTRING("Media"),
                                     mDocument,
                                     nsContentUtils::eDOM_PROPERTIES,
-                                    aReportStringId,
+                                    aNotification.mReportStringId,
                                     aParams.IsEmpty() ? nullptr : params,
                                     aParams.IsEmpty() ? 0 : 1);
   }
 
   // "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, aReportStringId)) {
+      || StringListContains(filter, aNotification.mReportStringId)) {
     DispatchNotification(
-      mDocument->GetInnerWindow(),
-      aNotificationType, aIsSolved, aReportStringId, aParams);
+      mDocument->GetInnerWindow(), aNotification, aIsSolved, aParams);
   }
 }
 
 enum SilverlightPresence {
   eNoSilverlight,
   eSilverlightDisabled,
   eSilverlightEnabled
 };
@@ -552,18 +570,17 @@ DecoderDoctorDocumentWatcher::Synthesize
   if (!unsupportedKeySystems.IsEmpty() && supportedKeySystems.IsEmpty()) {
     // No supported key systems!
     switch (lastKeySystemIssue) {
       case DecoderDoctorDiagnostics::eWidevineWithNoWMF:
         if (CheckSilverlight() != eSilverlightEnabled) {
           DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - unsupported key systems: %s, widevine without WMF nor Silverlight",
                    this, mDocument, NS_ConvertUTF16toUTF8(unsupportedKeySystems).get());
           ReportAnalysis(
-            dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
-            false, "MediaWidevineNoWMFNoSilverlight", unsupportedKeySystems);
+            sMediaWidevineNoWMFNoSilverlight, false, unsupportedKeySystems);
           return;
         }
         break;
       default:
         break;
     }
   }
 
@@ -572,43 +589,39 @@ DecoderDoctorDocumentWatcher::Synthesize
     // Some requested formats cannot be played.
     if (playableFormats.IsEmpty()) {
       // No requested formats can be played. See if we can help the user, by
       // going through expected decoders from most to least desirable.
 #if defined(XP_WIN)
       if (!formatsRequiringWMF.IsEmpty()) {
         DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - unplayable formats: %s -> Cannot play media because WMF was not found",
                  this, mDocument, NS_ConvertUTF16toUTF8(formatsRequiringWMF).get());
-        ReportAnalysis(dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
-                       false, "MediaWMFNeeded", formatsRequiringWMF);
+        ReportAnalysis(sMediaWMFNeeded, false, formatsRequiringWMF);
         return;
       }
 #endif
 #if defined(MOZ_FFMPEG)
       if (!formatsRequiringFFMpeg.IsEmpty()) {
         DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - unplayable formats: %s -> Cannot play media because platform decoder was not found",
                  this, mDocument, NS_ConvertUTF16toUTF8(formatsRequiringFFMpeg).get());
-        ReportAnalysis(dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
-                       "MediaPlatformDecoderNotFound", formatsRequiringFFMpeg);
+        ReportAnalysis(sMediaPlatformDecoderNotFound,
+                       false, formatsRequiringFFMpeg);
         return;
       }
 #endif
       DD_WARN("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Cannot play media, unplayable formats: %s",
               this, mDocument, NS_ConvertUTF16toUTF8(unplayableFormats).get());
-      ReportAnalysis(dom::DecoderDoctorNotificationType::Cannot_play,
-                     false, "MediaCannotPlayNoDecoders", unplayableFormats);
+      ReportAnalysis(sMediaCannotPlayNoDecoders, false, unplayableFormats);
       return;
     }
 
     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(
-        dom::DecoderDoctorNotificationType::Can_play_but_some_missing_decoders,
-        false, "MediaNoDecoders", unplayableFormats);
+      ReportAnalysis(sMediaNoDecoders, false, unplayableFormats);
     }
     return;
   }
   DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Can play media, decoders available for all requested formats",
            this, mDocument);
 }
 
 void