Bug 1686696 - part3 : enforce to only dispatch periodic `timeupdate` once within 250ms. r=bryce

Inplemented an event runner for `timeupdate` only, which would ensure that for periodic `timeupdate` it should be only dispatched once within 250ms.

Differential Revision: https://phabricator.services.mozilla.com/D102675

dom/html/HTMLMediaElement.cpp

@@ -6045,10 +6045,12 @@ void HTMLMediaElement::PrincipalHandleChangedForVideoFrameContainer(
 }
 
 already_AddRefed<nsMediaEventRunner> HTMLMediaElement::GetEventRunner(
-    const nsAString& aName) {
+    const nsAString& aName, EventFlag aFlag) {
   RefPtr<nsMediaEventRunner> runner;
   if (aName.EqualsLiteral("playing")) {
     runner = new nsNotifyAboutPlayingRunner(this, TakePendingPlayPromises());
+  } else if (aName.EqualsLiteral("timeupdate")) {
+    runner = new nsTimeupdateRunner(this, aFlag == EventFlag::eMandatory);
   } else {
     runner = new nsAsyncEventRunner(aName, this);
   }
@@ -6071,18 +6073,19 @@ nsresult HTMLMediaElement::DispatchEvent(const nsAString& aName) {
 }
 
 void HTMLMediaElement::DispatchAsyncEvent(const nsAString& aName) {
-  LOG_EVENT(LogLevel::Debug,
-            ("%p Queuing event %s", this, NS_ConvertUTF16toUTF8(aName).get()));
-  DDLOG(DDLogCategory::Event, "HTMLMediaElement",
-        nsCString(NS_ConvertUTF16toUTF8(aName)));
-
   RefPtr<nsMediaEventRunner> runner = GetEventRunner(aName);
+  DispatchAsyncEvent(std::move(runner));
+}
+
+void HTMLMediaElement::DispatchAsyncEvent(RefPtr<nsMediaEventRunner> aRunner) {
+  NS_ConvertUTF16toUTF8 eventName(aRunner->EventName());
+  LOG_EVENT(LogLevel::Debug, ("%p Queuing event %s", this, eventName.get()));
+  DDLOG(DDLogCategory::Event, "HTMLMediaElement", nsCString(eventName.get()));
   if (mEventBlocker->ShouldBlockEventDelivery()) {
-    mEventBlocker->PostponeEvent(runner);
+    mEventBlocker->PostponeEvent(aRunner);
     return;
   }
-
-  mMainThreadEventTarget->Dispatch(runner.forget());
+  mMainThreadEventTarget->Dispatch(aRunner.forget());
 }
 
 bool HTMLMediaElement::IsPotentiallyPlaying() const {
@@ -6464,6 +6467,16 @@ void HTMLMediaElement::SetRequestHeaders(nsIHttpChannel* aChannel) {
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
+const TimeStamp& HTMLMediaElement::LastTimeupdateDispatchTime() const {
+  MOZ_ASSERT(NS_IsMainThread());
+  return mLastTimeUpdateDispatchTime;
+}
+
+void HTMLMediaElement::UpdateLastTimeupdateDispatchTime() {
+  MOZ_ASSERT(NS_IsMainThread());
+  mLastTimeUpdateDispatchTime = TimeStamp::Now();
+}
+
 bool HTMLMediaElement::ShouldQueueTimeupdateAsyncTask(
     TimeupdateType aType) const {
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
@@ -6491,7 +6504,11 @@ void HTMLMediaElement::FireTimeUpdate(TimeupdateType aType) {
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
 
   if (ShouldQueueTimeupdateAsyncTask(aType)) {
-    DispatchAsyncEvent(u"timeupdate"_ns);
+    RefPtr<nsMediaEventRunner> runner =
+        GetEventRunner(u"timeupdate"_ns, aType == TimeupdateType::eMandatory
+                                             ? EventFlag::eMandatory
+                                             : EventFlag::eNone);
+    DispatchAsyncEvent(std::move(runner));
     mQueueTimeUpdateRunnerTime = TimeStamp::Now();
     mLastCurrentTime = CurrentTime();
   }

dom/html/HTMLMediaElement.h

@@ -160,6 +160,14 @@ class HTMLMediaElement : public nsGenericHTMLElement,
     ePeriodic = true,
   };
 
+  // This is used for event runner creation. Currently only timeupdate needs
+  // that, but it can be used to extend for other events in the future if
+  // necessary.
+  enum class EventFlag : uint8_t {
+    eNone = 0,
+    eMandatory = 1,
+  };
+
   /**
    * 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
@@ -294,6 +302,7 @@ class HTMLMediaElement : public nsGenericHTMLElement,
 
   // Dispatch events
   void DispatchAsyncEvent(const nsAString& aName) final;
+  void DispatchAsyncEvent(RefPtr<nsMediaEventRunner> aRunner);
 
   // Triggers a recomputation of readyState.
   void UpdateReadyState() override {
@@ -453,6 +462,9 @@ class HTMLMediaElement : public nsGenericHTMLElement,
     FireTimeUpdate(TimeupdateType::ePeriodic);
   }
 
+  const TimeStamp& LastTimeupdateDispatchTime() const;
+  void UpdateLastTimeupdateDispatchTime();
+
   // WebIDL
 
   MediaError* GetError() const;
@@ -1282,7 +1294,8 @@ class HTMLMediaElement : public nsGenericHTMLElement,
   // For nsAsyncEventRunner.
   nsresult DispatchEvent(const nsAString& aName);
 
-  already_AddRefed<nsMediaEventRunner> GetEventRunner(const nsAString& aName);
+  already_AddRefed<nsMediaEventRunner> GetEventRunner(
+      const nsAString& aName, EventFlag aFlag = EventFlag::eNone);
 
   // This method moves the mPendingPlayPromises into a temperate object. So the
   // mPendingPlayPromises is cleared after this method call.
@@ -1533,6 +1546,10 @@ class HTMLMediaElement : public nsGenericHTMLElement,
   // main thread only.
   TimeStamp mQueueTimeUpdateRunnerTime;
 
+  // Time that the last timeupdate event was fired. Read/Write from the
+  // main thread only.
+  TimeStamp mLastTimeUpdateDispatchTime;
+
   // Time that the last progress event was fired. Read/Write from the
   // main thread only.
   TimeStamp mProgressTime;

dom/media/utils/MediaElementEventRunners.cpp

@@ -114,5 +114,30 @@ NS_IMETHODIMP nsSyncSection::Run() {
   return NS_OK;
 }
 
+NS_IMETHODIMP nsTimeupdateRunner::Run() {
+  if (IsCancelled() || !ShouldDispatchTimeupdate()) {
+    return NS_OK;
+  }
+  // TODO (bug 1688128) : update time after we dispatch the event. Although
+  // current behavior matches the spec closely, but doing that would avoid the
+  // issue where timeupdate event listener takes lots of time and then we end
+  // up spending all time handling just timeupdate events.
+  mElement->UpdateLastTimeupdateDispatchTime();
+  return DispatchEvent(mEventName);
+}
+
+bool nsTimeupdateRunner::ShouldDispatchTimeupdate() const {
+  if (mIsMandatory) {
+    return true;
+  }
+
+  // If the main thread is busy, tasks may be delayed and dispatched at
+  // unexpected times. Ensure we don't dispatch `timeupdate` more often
+  // than once per `TIMEUPDATE_MS`.
+  const TimeStamp& lastTime = mElement->LastTimeupdateDispatchTime();
+  return lastTime.IsNull() || TimeStamp::Now() - lastTime >
+                                  TimeDuration::FromMilliseconds(TIMEUPDATE_MS);
+}
+
 #undef LOG_EVENT
 }  // namespace mozilla::dom

dom/media/utils/MediaElementEventRunners.h

@@ -184,6 +184,24 @@ class nsSyncSection : public nsMediaEventRunner {
   nsCOMPtr<nsIRunnable> mRunnable;
 };
 
+/**
+ * This runner is used to dispatch `timeupdate` event and ensure we don't
+ * dispatch `timeupdate` more often than once per `TIMEUPDATE_MS` if that is not
+ * a mandatory event.
+ */
+class nsTimeupdateRunner : public nsMediaEventRunner {
+ public:
+  nsTimeupdateRunner(HTMLMediaElement* aElement, bool aIsMandatory)
+      : nsMediaEventRunner(u"nsTimeupdateRunner"_ns, aElement,
+                           u"timeupdate"_ns),
+        mIsMandatory(aIsMandatory) {}
+  NS_IMETHOD Run() override;
+
+ private:
+  bool ShouldDispatchTimeupdate() const;
+  bool mIsMandatory;
+};
+
 }  // namespace dom
 }  // namespace mozilla