Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 22 Apr 2016 13:42:11 +1000
changeset 332306 c2c86d1fa8a140e6fc182453bca2f18864d85541
parent 332305 8deda08fc04b19c6af99569dcc0c6c767c8c3cd5
child 332307 cf1b1f91f522f42c5987a95b4960a7c13431373d
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs848994
milestone48.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 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