Bug 1530862 - Add an Init() method to HTMLMediaElement to be called right after construction to do any AddRef / Release-ing. r=jya,mccr8,smaug
☠☠ backed out by de132128473b ☠ ☠
authorMike Conley <mconley@mozilla.com>
Wed, 27 Feb 2019 23:40:00 +0000
changeset 519473 28bc841f06fc5f4d01defbacc80e017f701a8d57
parent 519472 37f52c22967c6e4354ca3fdb1599970ffe30e8c8
child 519474 5570cf4346a807801c535758f1b82dae1ca63f3f
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya, mccr8, smaug
bugs1530862
milestone67.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 1530862 - Add an Init() method to HTMLMediaElement to be called right after construction to do any AddRef / Release-ing. r=jya,mccr8,smaug Differential Revision: https://phabricator.services.mozilla.com/D21400
dom/html/HTMLAudioElement.cpp
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/html/HTMLVideoElement.cpp
dom/html/HTMLVideoElement.h
layout/build/nsLayoutStatics.cpp
--- a/dom/html/HTMLAudioElement.cpp
+++ b/dom/html/HTMLAudioElement.cpp
@@ -14,23 +14,31 @@
 #include "nsContentUtils.h"
 #include "nsJSUtils.h"
 #include "AudioSampleFormat.h"
 #include <algorithm>
 #include "nsComponentManagerUtils.h"
 #include "nsIHttpChannel.h"
 #include "mozilla/dom/TimeRanges.h"
 #include "AudioStream.h"
+#include "mozilla/Unused.h"
 
-NS_IMPL_NS_NEW_HTML_ELEMENT(Audio)
+nsGenericHTMLElement* NS_NewHTMLAudioElement(
+    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
+    mozilla::dom::FromParser aFromParser) {
+  mozilla::dom::HTMLAudioElement* element =
+      new mozilla::dom::HTMLAudioElement(std::move(aNodeInfo));
+  mozilla::Unused << element->Init();
+  return element;
+}
 
 namespace mozilla {
 namespace dom {
 
-NS_IMPL_ELEMENT_CLONE(HTMLAudioElement)
+NS_IMPL_ELEMENT_CLONE_WITH_INIT(HTMLAudioElement)
 
 HTMLAudioElement::HTMLAudioElement(already_AddRefed<NodeInfo>&& aNodeInfo)
     : HTMLMediaElement(std::move(aNodeInfo)) {
   DecoderDoctorLogger::LogConstruction(this);
 }
 
 HTMLAudioElement::~HTMLAudioElement() {
   DecoderDoctorLogger::LogDestruction(this);
@@ -49,17 +57,18 @@ already_AddRefed<HTMLAudioElement> HTMLA
   if (!win || !(doc = win->GetExtantDoc())) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   RefPtr<mozilla::dom::NodeInfo> nodeInfo = doc->NodeInfoManager()->GetNodeInfo(
       nsGkAtoms::audio, nullptr, kNameSpaceID_XHTML, ELEMENT_NODE);
 
-  RefPtr<HTMLAudioElement> audio = new HTMLAudioElement(nodeInfo.forget());
+  RefPtr<HTMLAudioElement> audio =
+      static_cast<HTMLAudioElement*>(NS_NewHTMLAudioElement(nodeInfo.forget()));
   audio->SetHTMLAttr(nsGkAtoms::preload, NS_LITERAL_STRING("auto"), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
   if (aSrc.WasPassed()) {
     audio->SetSrc(aSrc.Value(), aRv);
   }
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -3493,23 +3493,37 @@ HTMLMediaElement::HTMLMediaElement(
       mWatchManager(this,
                     OwnerDoc()->AbstractMainThreadFor(TaskCategory::Other)),
       mMainThreadEventTarget(OwnerDoc()->EventTargetFor(TaskCategory::Other)),
       mAbstractMainThread(
           OwnerDoc()->AbstractMainThreadFor(TaskCategory::Other)),
       mShutdownObserver(new ShutdownObserver),
       mPlayed(new TimeRanges(ToSupports(OwnerDoc()))),
       mPaused(true, "HTMLMediaElement::mPaused"),
-      mAudioTrackList(new AudioTrackList(OwnerDoc()->GetParentObject(), this)),
-      mVideoTrackList(new VideoTrackList(OwnerDoc()->GetParentObject(), this)),
       mErrorSink(new ErrorSink(this)),
       mAudioChannelWrapper(new AudioChannelAgentCallback(this)),
       mSink(MakePair(nsString(), RefPtr<AudioDeviceInfo>())) {
   MOZ_ASSERT(mMainThreadEventTarget);
   MOZ_ASSERT(mAbstractMainThread);
+  // Please don't add anything to this constructor or the initialization
+  // list that can cause AddRef to be called. This prevents subclasses
+  // from overriding AddRef in a way that works with our refcount
+  // logging mechanisms. Put these things inside of the ::Init method
+  // instead.
+}
+
+nsresult HTMLMediaElement::Init() {
+  MOZ_ASSERT(mRefCnt == 0 && !mRefCnt.IsPurple(),
+             "HTMLMediaElement::Init called when AddRef has been called "
+             "at least once already, probably in the constructor. Please "
+             "see the documentation in the HTMLMediaElement constructor.");
+  MOZ_ASSERT(!mRefCnt.IsPurple());
+
+  mAudioTrackList = new AudioTrackList(OwnerDoc()->GetParentObject(), this);
+  mVideoTrackList = new VideoTrackList(OwnerDoc()->GetParentObject(), this);
 
   DecoderDoctorLogger::LogConstruction(this);
 
   mWatchManager.Watch(mPaused, &HTMLMediaElement::UpdateWakeLock);
 
   ErrorResult rv;
 
   double defaultVolume = Preferences::GetFloat("media.default_volume", 1.0);
@@ -3520,19 +3534,23 @@ HTMLMediaElement::HTMLMediaElement(
 
   // We initialize the MediaShutdownManager as the HTMLMediaElement is always
   // constructed on the main thread, and not during stable state.
   // (MediaShutdownManager make use of nsIAsyncShutdownClient which is written
   // in JS)
   MediaShutdownManager::InitStatics();
 
   mShutdownObserver->Subscribe(this);
+  mInitialized = true;
+  return NS_OK;
 }
 
 HTMLMediaElement::~HTMLMediaElement() {
+  MOZ_ASSERT(mInitialized,
+             "HTMLMediaElement must be initialized before it is destroyed.");
   NS_ASSERTION(
       !mHasSelfReference,
       "How can we be destroyed if we're still holding a self reference?");
 
   mShutdownObserver->Unsubscribe();
 
   if (mVideoFrameContainer) {
     mVideoFrameContainer->ForgetElement();
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -112,16 +112,17 @@ class HTMLMediaElement : public nsGeneri
 
   MOZ_DECLARE_WEAKREFERENCE_TYPENAME(HTMLMediaElement)
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
 
   CORSMode GetCORSMode() { return mCORSMode; }
 
   explicit HTMLMediaElement(
       already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
+  nsresult Init();
 
   void ReportCanPlayTelemetry();
 
   /**
    * This is used when the browser is constructing a video element to play
    * a channel that we've already started loading. The src attribute and
    * <source> children are ignored.
    * @param aChannel the channel to use
@@ -1745,16 +1746,19 @@ class HTMLMediaElement : public nsGeneri
 
   // True if media has ever been blocked by autoplay policy before.
   bool mHasPlayEverBeenBlocked = false;
 
   // Report the Telemetry about whether media played over the specific time
   // threshold.
   void ReportPlayedTimeAfterBlockedTelemetry();
 
+  // True if Init() has been called after construction
+  bool mInitialized = false;
+
   // True if user has called load(), seek() or element has started playing
   // before. It's *only* use for checking autoplay policy
   bool mIsBlessed = false;
 
   // True if the first frame has been successfully loaded.
   bool mFirstFrameLoaded = false;
 
   // Media elements also have a default playback start position, which must
--- a/dom/html/HTMLVideoElement.cpp
+++ b/dom/html/HTMLVideoElement.cpp
@@ -26,28 +26,36 @@
 #include "MediaError.h"
 #include "MediaDecoder.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/WakeLock.h"
 #include "mozilla/dom/power/PowerManagerService.h"
 #include "mozilla/dom/Performance.h"
 #include "mozilla/dom/TimeRanges.h"
 #include "mozilla/dom/VideoPlaybackQuality.h"
+#include "mozilla/Unused.h"
 
 #include <algorithm>
 #include <limits>
 
-NS_IMPL_NS_NEW_HTML_ELEMENT(Video)
+nsGenericHTMLElement* NS_NewHTMLVideoElement(
+    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
+    mozilla::dom::FromParser aFromParser) {
+  mozilla::dom::HTMLVideoElement* element =
+      new mozilla::dom::HTMLVideoElement(std::move(aNodeInfo));
+  mozilla::Unused << element->Init();
+  return element;
+}
 
 namespace mozilla {
 namespace dom {
 
 static bool sVideoStatsEnabled;
 
-NS_IMPL_ELEMENT_CLONE(HTMLVideoElement)
+NS_IMPL_ELEMENT_CLONE_WITH_INIT(HTMLVideoElement)
 
 HTMLVideoElement::HTMLVideoElement(already_AddRefed<NodeInfo>&& aNodeInfo)
     : HTMLMediaElement(std::move(aNodeInfo)), mIsOrientationLocked(false) {
   DecoderDoctorLogger::LogConstruction(this);
 }
 
 HTMLVideoElement::~HTMLVideoElement() {
   DecoderDoctorLogger::LogDestruction(this);
@@ -303,17 +311,18 @@ void HTMLVideoElement::ReleaseVideoWakeL
     ErrorResult rv;
     mScreenWakeLock->Unlock(rv);
     rv.SuppressException();
     mScreenWakeLock = nullptr;
     return;
   }
 }
 
-void HTMLVideoElement::Init() {
+/* static */
+void HTMLVideoElement::InitStatics() {
   Preferences::AddBoolVarCache(&sVideoStatsEnabled,
                                "media.video_stats.enabled");
 }
 
 /* static */
 bool HTMLVideoElement::IsVideoStatsEnabled() { return sVideoStatsEnabled; }
 
 double HTMLVideoElement::TotalPlayTime() const {
--- a/dom/html/HTMLVideoElement.h
+++ b/dom/html/HTMLVideoElement.h
@@ -33,17 +33,17 @@ class HTMLVideoElement final : public HT
   virtual bool IsVideo() const override { return true; }
 
   virtual bool ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute,
                               const nsAString& aValue,
                               nsIPrincipal* aMaybeScriptedPrincipal,
                               nsAttrValue& aResult) override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
 
-  static void Init();
+  static void InitStatics();
 
   virtual nsMapRuleToAttributesFunc GetAttributeMappingFunction()
       const override;
 
   virtual nsresult Clone(NodeInfo*, nsINode** aResult) const override;
 
   // Set size with the current video frame's height and width.
   // If there is no video frame, returns NS_ERROR_FAILURE.
--- a/layout/build/nsLayoutStatics.cpp
+++ b/layout/build/nsLayoutStatics.cpp
@@ -250,17 +250,17 @@ nsresult nsLayoutStatics::Initialize() {
   nsSVGUtils::Init();
 
   ProcessPriorityManager::Init();
 
   nsPermissionManager::ClearOriginDataObserverInit();
   nsCookieService::AppClearDataObserverInit();
   nsApplicationCacheService::AppClearDataObserverInit();
 
-  HTMLVideoElement::Init();
+  HTMLVideoElement::InitStatics();
   nsGenericHTMLFrameElement::InitStatics();
 
 #ifdef MOZ_XUL
   nsMenuBarListener::InitializeStatics();
 #endif
 
   CacheObserver::Init();