Bug 1271483 - p14. Demagicify ReportStringId strings - r=cpearce
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 25 May 2016 01:03:21 +1000
changeset 302285 d780f0ca52436491f471b24bb99a22222aa9c870
parent 302284 2bc76d3f7827576cd8d24e0dee225fc0d61dd2f4
child 302286 575198d83ce778cd2dad9e7ce6978b6f58937738
push id78676
push usergsquelart@mozilla.com
push dateWed, 22 Jun 2016 03:54:32 +0000
treeherdermozilla-inbound@da005aa1d83c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1271483
milestone50.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 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