Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 22 Apr 2016 11:48:28 +1000
changeset 355182 65499eec533972a21885a71365aa043e92aed53f
parent 355133 0891f0fa044cba28024849803e170ed7700e01e0
child 355183 f2506b33d79174616120f8413b7d890871bf239a
push id16221
push usergsquelart@mozilla.com
push dateFri, 22 Apr 2016 01:56:41 +0000
reviewerscpearce
bugs848994
milestone48.0a1
Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce Some refactoring, clean-ups, etc. The main change is that the can-play status is passed when diagnostics are finally recorded. This will help when introducing different types of diagnostics in future patches (e.g., Key System issues). MozReview-Commit-ID: 182ePlrMqn4
dom/html/HTMLMediaElement.cpp
dom/media/DecoderDoctorDiagnostics.cpp
dom/media/DecoderDoctorDiagnostics.h
dom/media/mediasource/MediaSource.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -1060,20 +1060,18 @@ void HTMLMediaElement::LoadFromSourceChi
       continue;
     }
 
     // If we have a type attribute, it must be a supported type.
     nsAutoString type;
     if (child->GetAttr(kNameSpaceID_None, nsGkAtoms::type, type)) {
       DecoderDoctorDiagnostics diagnostics;
       CanPlayStatus canPlay = GetCanPlay(type, &diagnostics);
-      if (canPlay != CANPLAY_NO) {
-        diagnostics.SetCanPlay();
-      }
-      diagnostics.StoreDiagnostics(OwnerDoc(), type, __func__);
+      diagnostics.StoreFormatDiagnostics(
+        OwnerDoc(), type, canPlay != CANPLAY_NO, __func__);
       if (canPlay == CANPLAY_NO) {
         DispatchAsyncSourceError(child);
         const char16_t* params[] = { type.get(), src.get() };
         ReportLoadError("MediaLoadUnsupportedTypeAttribute", params, ArrayLength(params));
         continue;
       }
     }
     nsAutoString media;
@@ -2911,20 +2909,18 @@ HTMLMediaElement::GetCanPlay(const nsASt
                                            aDiagnostics);
 }
 
 NS_IMETHODIMP
 HTMLMediaElement::CanPlayType(const nsAString& aType, nsAString& aResult)
 {
   DecoderDoctorDiagnostics diagnostics;
   CanPlayStatus canPlay = GetCanPlay(aType, &diagnostics);
-  if (canPlay != CANPLAY_NO) {
-    diagnostics.SetCanPlay();
-  }
-  diagnostics.StoreDiagnostics(OwnerDoc(), aType, __func__);
+  diagnostics.StoreFormatDiagnostics(
+    OwnerDoc(), aType, canPlay != CANPLAY_NO, __func__);
   switch (canPlay) {
   case CANPLAY_NO:
     aResult.Truncate();
     break;
   case CANPLAY_YES:
     aResult.AssignLiteral("probably");
     break;
   default:
@@ -2978,22 +2974,20 @@ nsresult HTMLMediaElement::InitializeDec
   nsAutoCString mimeType;
 
   aChannel->GetContentType(mimeType);
   NS_ASSERTION(!mimeType.IsEmpty(), "We should have the Content-Type.");
 
   DecoderDoctorDiagnostics diagnostics;
   RefPtr<MediaDecoder> decoder =
     DecoderTraits::CreateDecoder(mimeType, this, &diagnostics);
-  if (decoder) {
-    diagnostics.SetCanPlay();
-  }
-  diagnostics.StoreDiagnostics(OwnerDoc(),
-                               NS_ConvertASCIItoUTF16(mimeType),
-                               __func__);
+  diagnostics.StoreFormatDiagnostics(OwnerDoc(),
+                                     NS_ConvertASCIItoUTF16(mimeType),
+                                     decoder != nullptr,
+                                     __func__);
   if (!decoder) {
     nsAutoString src;
     GetCurrentSrc(src);
     NS_ConvertUTF8toUTF16 mimeUTF16(mimeType);
     const char16_t* params[] = { mimeUTF16.get(), src.get() };
     ReportLoadError("MediaLoadUnsupportedMimeType", params, ArrayLength(params));
     return NS_ERROR_FAILURE;
   }
--- a/dom/media/DecoderDoctorDiagnostics.cpp
+++ b/dom/media/DecoderDoctorDiagnostics.cpp
@@ -30,25 +30,24 @@ namespace mozilla
 // 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
 // destroyed once all references are gone.
 class DecoderDoctorDocumentWatcher : public nsITimerCallback
 {
 public:
-  static RefPtr<DecoderDoctorDocumentWatcher>
+  static already_AddRefed<DecoderDoctorDocumentWatcher>
   RetrieveOrCreate(nsIDocument* aDocument);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSITIMERCALLBACK
 
-  void AddDiagnostics(const nsAString& aFormat,
-                      const char* aCallSite,
-                      DecoderDoctorDiagnostics&& aDiagnostics);
+  void AddDiagnostics(DecoderDoctorDiagnostics&& aDiagnostics,
+                      const char* aCallSite);
 
 private:
   explicit DecoderDoctorDocumentWatcher(nsIDocument* aDocument);
   virtual ~DecoderDoctorDocumentWatcher();
 
   // This will prevent further work from happening, watcher will deregister
   // itself from document (if requested) and cancel any timer, and soon die.
   void StopWatching(bool aRemoveProperty);
@@ -80,45 +79,41 @@ private:
   //    period, so we just stop watching.
   // Once nulled, no more actual work will happen, and the watcher will be
   // destroyed soon.
   nsIDocument* mDocument;
 
   struct Diagnostics
   {
     Diagnostics(DecoderDoctorDiagnostics&& aDiagnostics,
-                const nsAString& aFormat,
                 const char* aCallSite)
       : mDecoderDoctorDiagnostics(Move(aDiagnostics))
-      , mFormat(aFormat)
       , mCallSite(aCallSite)
     {}
     Diagnostics(const Diagnostics&) = delete;
     Diagnostics(Diagnostics&& aOther)
       : mDecoderDoctorDiagnostics(Move(aOther.mDecoderDoctorDiagnostics))
-      , mFormat(Move(aOther.mFormat))
       , mCallSite(Move(aOther.mCallSite))
     {}
 
     const DecoderDoctorDiagnostics mDecoderDoctorDiagnostics;
-    const nsString mFormat;
     const nsCString mCallSite;
   };
   typedef nsTArray<Diagnostics> DiagnosticsSequence;
   DiagnosticsSequence mDiagnosticsSequence;
 
   nsCOMPtr<nsITimer> mTimer; // Keep timer alive until we run.
   DiagnosticsSequence::size_type mDiagnosticsHandled = 0;
 };
 
 
 NS_IMPL_ISUPPORTS(DecoderDoctorDocumentWatcher, nsITimerCallback)
 
 // static
-RefPtr<DecoderDoctorDocumentWatcher>
+already_AddRefed<DecoderDoctorDocumentWatcher>
 DecoderDoctorDocumentWatcher::RetrieveOrCreate(nsIDocument* aDocument)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aDocument);
   RefPtr<DecoderDoctorDocumentWatcher> watcher =
     static_cast<DecoderDoctorDocumentWatcher*>(
       aDocument->GetProperty(nsGkAtoms::decoderDoctor));
   if (!watcher) {
@@ -131,17 +126,17 @@ DecoderDoctorDocumentWatcher::RetrieveOr
       DD_WARN("DecoderDoctorDocumentWatcher::RetrieveOrCreate(doc=%p) - Could not set property in document, will destroy new watcher[%p]",
               aDocument, watcher.get());
       return nullptr;
     }
     // Document owns watcher through this property.
     // Released in DestroyPropertyCallback().
     NS_ADDREF(watcher.get());
   }
-  return watcher;
+  return watcher.forget();
 }
 
 DecoderDoctorDocumentWatcher::DecoderDoctorDocumentWatcher(nsIDocument* aDocument)
   : mDocument(aDocument)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mDocument);
   DD_DEBUG("DecoderDoctorDocumentWatcher[%p]::DecoderDoctorDocumentWatcher(doc=%p)",
@@ -300,79 +295,81 @@ DecoderDoctorDocumentWatcher::ReportAnal
 
 void
 DecoderDoctorDocumentWatcher::SynthesizeAnalysis()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   bool canPlay = false;
 #if defined(MOZ_FFMPEG)
-  bool PlatformDecoderNeeded = false;
+  bool FFMpegNeeded = false;
 #endif
-  nsAutoString formats;
+  nsAutoString unplayableFormats;
+
   for (auto& diag : mDiagnosticsSequence) {
-    if (diag.mDecoderDoctorDiagnostics.CanPlay()) {
-      canPlay = true;
-    } else {
+    if (!diag.mDecoderDoctorDiagnostics.Format().IsEmpty()) {
+      if (diag.mDecoderDoctorDiagnostics.CanPlay()) {
+        canPlay = true;
+      } else {
 #if defined(MOZ_FFMPEG)
-      if (diag.mDecoderDoctorDiagnostics.DidFFmpegFailToLoad()) {
-        PlatformDecoderNeeded = true;
-      }
+        if (diag.mDecoderDoctorDiagnostics.DidFFmpegFailToLoad()) {
+          FFMpegNeeded = true;
+        }
 #endif
-      if (!formats.IsEmpty()) {
-        formats += NS_LITERAL_STRING(", ");
+        if (!unplayableFormats.IsEmpty()) {
+          unplayableFormats += NS_LITERAL_STRING(", ");
+        }
+        unplayableFormats += diag.mDecoderDoctorDiagnostics.Format();
       }
-      formats += diag.mFormat;
     }
   }
+
   if (!canPlay) {
 #if defined(MOZ_FFMPEG)
-    if (PlatformDecoderNeeded) {
-      DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::Notify() - formats: %s -> Cannot play media because platform decoder was not found",
-               this, mDocument, NS_ConvertUTF16toUTF8(formats).get());
+    if (FFMpegNeeded) {
+      DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - unplayable formats: %s -> Cannot play media because platform decoder was not found",
+               this, mDocument, NS_ConvertUTF16toUTF8(unplayableFormats).get());
       ReportAnalysis(dom::DecoderDoctorNotificationType::Platform_decoder_not_found,
-                     "MediaPlatformDecoderNotFound", formats);
+                     "MediaPlatformDecoderNotFound", unplayableFormats);
     } else
 #endif
     {
-      DD_WARN("DecoderDoctorDocumentWatcher[%p, doc=%p]::Notify() - Cannot play media, formats: %s",
-              this, mDocument, NS_ConvertUTF16toUTF8(formats).get());
+      DD_WARN("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Cannot play media, unplayable formats: %s",
+              this, mDocument, NS_ConvertUTF16toUTF8(unplayableFormats).get());
       ReportAnalysis(dom::DecoderDoctorNotificationType::Cannot_play,
-                     "MediaCannotPlayNoDecoders", formats);
+                     "MediaCannotPlayNoDecoders", unplayableFormats);
     }
-  } else if (!formats.IsEmpty()) {
-    DD_INFO("DecoderDoctorDocumentWatcher[%p, doc=%p]::Notify() - Can play media, but no decoders for some requested formats: %s",
-            this, mDocument, NS_ConvertUTF16toUTF8(formats).get());
+  } else if (!unplayableFormats.IsEmpty()) {
+    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.decoderdoctor.verbose", false)) {
       ReportAnalysis(
         dom::DecoderDoctorNotificationType::Can_play_but_some_missing_decoders,
-        "MediaNoDecoders", formats);
+        "MediaNoDecoders", unplayableFormats);
     }
   } else {
-    DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::Notify() - Can play media, decoders available for all requested formats",
+    DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::SynthesizeAnalysis() - Can play media, decoders available for all requested formats",
              this, mDocument);
   }
 }
 
 void
-DecoderDoctorDocumentWatcher::AddDiagnostics(const nsAString& aFormat,
-                                            const char* aCallSite,
-                                            DecoderDoctorDiagnostics&& aDiagnostics)
+DecoderDoctorDocumentWatcher::AddDiagnostics(DecoderDoctorDiagnostics&& aDiagnostics,
+                                             const char* aCallSite)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mDocument) {
     return;
   }
 
-  DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::AddDiagnostics(format='%s', call site '%s', can play=%d, platform lib failed to load=%d)",
-           this, mDocument, NS_ConvertUTF16toUTF8(aFormat).get(),
-           aCallSite, aDiagnostics.CanPlay(), aDiagnostics.DidFFmpegFailToLoad());
+  DD_DEBUG("DecoderDoctorDocumentWatcher[%p, doc=%p]::AddDiagnostics(DecoderDoctorDiagnostics{%s}, call site '%s')",
+           this, mDocument, aDiagnostics.GetDescription().Data(), aCallSite);
   mDiagnosticsSequence.AppendElement(
-    Diagnostics(Move(aDiagnostics), aFormat, aCallSite));
+    Diagnostics(Move(aDiagnostics), aCallSite));
   EnsureTimerIsStarted();
 }
 
 NS_IMETHODIMP
 DecoderDoctorDocumentWatcher::Notify(nsITimer* timer)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(timer == mTimer);
@@ -402,34 +399,60 @@ DecoderDoctorDocumentWatcher::Notify(nsI
     StopWatching(true);
   }
 
   return NS_OK;
 }
 
 
 void
-DecoderDoctorDiagnostics::StoreDiagnostics(nsIDocument* aDocument,
-                                           const nsAString& aFormat,
-                                           const char* aCallSite)
+DecoderDoctorDiagnostics::StoreFormatDiagnostics(nsIDocument* aDocument,
+                                                 const nsAString& aFormat,
+                                                 bool aCanPlay,
+                                                 const char* aCallSite)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (NS_WARN_IF(!aDocument)) {
-    DD_WARN("DecoderDoctorDiagnostics[%p]::StoreDiagnostics(nsIDocument* aDocument=nullptr, format='%s', call site '%s')",
-            this, NS_ConvertUTF16toUTF8(aFormat).get(), aCallSite);
+    DD_WARN("DecoderDoctorDiagnostics[%p]::StoreFormatDiagnostics(nsIDocument* aDocument=nullptr, format='%s', can-play=%d, call site '%s')",
+            this, NS_ConvertUTF16toUTF8(aFormat).get(), aCanPlay, aCallSite);
+    return;
+  }
+  if (NS_WARN_IF(aFormat.IsEmpty())) {
+    DD_WARN("DecoderDoctorDiagnostics[%p]::StoreFormatDiagnostics(nsIDocument* aDocument=%p, format=<empty>, can-play=%d, call site '%s')",
+            this, aDocument, aCanPlay, aCallSite);
     return;
   }
 
   RefPtr<DecoderDoctorDocumentWatcher> watcher =
     DecoderDoctorDocumentWatcher::RetrieveOrCreate(aDocument);
 
   if (NS_WARN_IF(!watcher)) {
-    DD_WARN("DecoderDoctorDiagnostics[%p]::StoreDiagnostics(nsIDocument* aDocument=nullptr, format='%s', call site '%s') - Could not create document watcher",
-            this, NS_ConvertUTF16toUTF8(aFormat).get(), aCallSite);
+    DD_WARN("DecoderDoctorDiagnostics[%p]::StoreFormatDiagnostics(nsIDocument* aDocument=%p, format='%s', can-play=%d, call site '%s') - Could not create document watcher",
+            this, aDocument, NS_ConvertUTF16toUTF8(aFormat).get(), aCanPlay, aCallSite);
     return;
   }
 
+  mFormat = aFormat;
+  mCanPlay = aCanPlay;
+
   // StoreDiagnostics should only be called once, after all data is available,
   // so it is safe to Move() from this object.
-  watcher->AddDiagnostics(aFormat, aCallSite, Move(*this));
+  watcher->AddDiagnostics(Move(*this), aCallSite);
+}
+
+nsCString
+DecoderDoctorDiagnostics::GetDescription() const
+{
+  nsCString s;
+  if (!mFormat.IsEmpty()) {
+    s = "format='";
+    s += NS_ConvertUTF16toUTF8(mFormat).get();
+    s += mCanPlay ? "', can play" : "', cannot play";
+    if (mFFmpegFailedToLoad) {
+      s += ", Linux platform decoder failed to load";
+    }
+  } else {
+    s = "?";
+  }
+  return s;
 }
 
 } // namespace mozilla
--- a/dom/media/DecoderDoctorDiagnostics.h
+++ b/dom/media/DecoderDoctorDiagnostics.h
@@ -30,30 +30,35 @@ namespace mozilla {
 
 class DecoderDoctorDiagnostics
 {
 public:
   // Store the diagnostic information collected so far on a document for a
   // given format. All diagnostics for a document will be analyzed together
   // within a short timeframe.
   // Should only be called once.
-  void StoreDiagnostics(nsIDocument* aDocument,
-                        const nsAString& aFormat,
-                        const char* aCallSite);
+  void StoreFormatDiagnostics(nsIDocument* aDocument,
+                              const nsAString& aFormat,
+                              bool aCanPlay,
+                              const char* aCallSite);
+
+  // Description string, for logging purposes.
+  nsCString GetDescription() const;
 
   // Methods to record diagnostic information:
 
-  void SetCanPlay() { mCanPlay = true; }
+  const nsAString& Format() const { return mFormat; }
   bool CanPlay() const { return mCanPlay; }
 
   void SetFFmpegFailedToLoad() { mFFmpegFailedToLoad = true; }
   bool DidFFmpegFailToLoad() const { return mFFmpegFailedToLoad; }
 
 private:
-  // True if there is at least one decoder that can play the media.
+  nsString mFormat;
+  // True if there is at least one decoder that can play that format.
   bool mCanPlay = false;
 
   bool mFFmpegFailedToLoad = false;
 };
 
 } // namespace mozilla
 
 #endif
--- a/dom/media/mediasource/MediaSource.cpp
+++ b/dom/media/mediasource/MediaSource.cpp
@@ -109,36 +109,30 @@ IsTypeSupported(const nsAString& aType, 
           return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
         }
         if (hasCodecs &&
             DecoderTraits::CanHandleCodecsType(mimeTypeUTF8.get(),
                                                codecs,
                                                aDiagnostics) == CANPLAY_NO) {
           return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
         }
-        if (aDiagnostics) {
-          aDiagnostics->SetCanPlay();
-        }
         return NS_OK;
       } else if (DecoderTraits::IsWebMTypeAndEnabled(mimeTypeUTF8)) {
         if (!(Preferences::GetBool("media.mediasource.webm.enabled", false) ||
               (Preferences::GetBool("media.mediasource.webm.audio.enabled", true) &&
                DecoderTraits::IsWebMAudioType(mimeTypeUTF8)) ||
               IsWebMForced(aDiagnostics))) {
           return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
         }
         if (hasCodecs &&
             DecoderTraits::CanHandleCodecsType(mimeTypeUTF8.get(),
                                                codecs,
                                                aDiagnostics) == CANPLAY_NO) {
           return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
         }
-        if (aDiagnostics) {
-          aDiagnostics->SetCanPlay();
-        }
         return NS_OK;
       }
     }
   }
 
   return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
 }
 
@@ -227,26 +221,20 @@ MediaSource::SetDuration(double aDuratio
 }
 
 already_AddRefed<SourceBuffer>
 MediaSource::AddSourceBuffer(const nsAString& aType, ErrorResult& aRv)
 {
   MOZ_ASSERT(NS_IsMainThread());
   DecoderDoctorDiagnostics diagnostics;
   nsresult rv = mozilla::IsTypeSupported(aType, &diagnostics);
-  if (NS_SUCCEEDED(rv)) {
-    diagnostics.SetCanPlay();
-  }
-  if (GetOwner()) {
-    diagnostics.StoreDiagnostics(GetOwner()->GetExtantDoc(), aType,
-                                 "AddSourceBuffer with owner window's doc");
-  } else {
-    diagnostics.StoreDiagnostics(nullptr, aType,
-                                 "AddSourceBuffer with nothing");
-  }
+  diagnostics.StoreFormatDiagnostics(GetOwner()
+                                     ? GetOwner()->GetExtantDoc()
+                                     : nullptr,
+                                     aType, NS_SUCCEEDED(rv), __func__);
   MSE_API("AddSourceBuffer(aType=%s)%s",
           NS_ConvertUTF16toUTF8(aType).get(),
           rv == NS_OK ? "" : " [not supported]");
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
     return nullptr;
   }
   if (mSourceBuffers->Length() >= MAX_SOURCE_BUFFERS) {
@@ -357,27 +345,19 @@ MediaSource::EndOfStream(const Optional<
 }
 
 /* static */ bool
 MediaSource::IsTypeSupported(const GlobalObject& aOwner, const nsAString& aType)
 {
   MOZ_ASSERT(NS_IsMainThread());
   DecoderDoctorDiagnostics diagnostics;
   nsresult rv = mozilla::IsTypeSupported(aType, &diagnostics);
-  if (NS_SUCCEEDED(rv)) {
-    diagnostics.SetCanPlay();
-  }
   nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(aOwner.GetAsSupports());
-  if (window) {
-    diagnostics.StoreDiagnostics(window->GetExtantDoc(), aType,
-                                 "IsTypeSupported with aOwner window's doc");
-  } else {
-    diagnostics.StoreDiagnostics(nullptr, aType,
-                                 "IsTypeSupported with nothing");
-  }
+  diagnostics.StoreFormatDiagnostics(window ? window->GetExtantDoc() : nullptr,
+                                     aType, NS_SUCCEEDED(rv), __func__);
 #define this nullptr
   MSE_API("IsTypeSupported(aType=%s)%s ",
           NS_ConvertUTF16toUTF8(aType).get(), rv == NS_OK ? "OK" : "[not supported]");
 #undef this // don't ever remove this line !
   return NS_SUCCEEDED(rv);
 }
 
 /* static */ bool