Bug 1359833 - Part 2. IDecodingTask should use the event target from ProgressTracker for main thread runnables. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 19 Jul 2017 14:15:11 -0400
changeset 369669 8941aa986abc5d3ba40d8f48d840da5bbf840df1
parent 369668 fd25a183897f665d70086ebd97033cb0f851c58c
child 369670 ca9a48e8a4fbfa5b236c7da73d20d626638f91ba
push id92726
push useraosmond@gmail.com
push dateWed, 19 Jul 2017 18:15:21 +0000
treeherdermozilla-inbound@09deb3ef1f63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1359833
milestone56.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 1359833 - Part 2. IDecodingTask should use the event target from ProgressTracker for main thread runnables. r=tnikkel
image/IDecodingTask.cpp
image/IDecodingTask.h
--- a/image/IDecodingTask.cpp
+++ b/image/IDecodingTask.cpp
@@ -8,95 +8,134 @@
 #include "gfxPrefs.h"
 #include "nsThreadUtils.h"
 
 #include "Decoder.h"
 #include "DecodePool.h"
 #include "RasterImage.h"
 #include "SurfaceCache.h"
 
+#include "mozilla/SystemGroup.h"
+
 namespace mozilla {
 
 using gfx::IntRect;
 
 namespace image {
 
 ///////////////////////////////////////////////////////////////////////////////
 // Helpers for sending notifications to the image associated with a decoder.
 ///////////////////////////////////////////////////////////////////////////////
 
-/* static */ void
+void
+IDecodingTask::EnsureHasEventTarget(NotNull<RasterImage*> aImage)
+{
+  if (!mEventTarget) {
+    // We determine the event target as late as possible, at the first dispatch
+    // time, because the observers bound to an imgRequest will affect it.
+    // We cache it rather than query for the event target each time because the
+    // event target can change. We don't want to risk events being executed in
+    // a different order than they are dispatched, which can happen if we
+    // selected scheduler groups which have no ordering guarantees relative to
+    // each other (e.g. it moves from scheduler group A for doc group DA to
+    // scheduler group B for doc group DB due to changing observers -- if we
+    // dispatched the first event on A, and the second on B, we don't know which
+    // will execute first.)
+    RefPtr<ProgressTracker> tracker = aImage->GetProgressTracker();
+    if (tracker) {
+      mEventTarget = tracker->GetEventTarget();
+    } else {
+      mEventTarget = SystemGroup::EventTargetFor(TaskCategory::Other);
+    }
+  }
+}
+
+bool
+IDecodingTask::IsOnEventTarget() const
+{
+  // This is essentially equivalent to NS_IsOnMainThread() because all of the
+  // event targets are for the main thread (although perhaps with a different
+  // label / scheduler group). The observers in ProgressTracker may have
+  // different event targets from this, so this is just a best effort guess.
+  bool current = false;
+  mEventTarget->IsOnCurrentThread(&current);
+  return current;
+}
+
+void
 IDecodingTask::NotifyProgress(NotNull<RasterImage*> aImage,
                               NotNull<Decoder*> aDecoder)
 {
   MOZ_ASSERT(aDecoder->HasProgress() && !aDecoder->IsMetadataDecode());
+  EnsureHasEventTarget(aImage);
 
   // Capture the decoder's state. If we need to notify asynchronously, it's
   // important that we don't wait until the lambda actually runs to capture the
   // state that we're going to notify. That would both introduce data races on
   // the decoder's state and cause inconsistencies between the NotifyProgress()
   // calls we make off-main-thread and the notifications that RasterImage
   // actually receives, which would cause bugs.
   Progress progress = aDecoder->TakeProgress();
   IntRect invalidRect = aDecoder->TakeInvalidRect();
   Maybe<uint32_t> frameCount = aDecoder->TakeCompleteFrameCount();
   DecoderFlags decoderFlags = aDecoder->GetDecoderFlags();
   SurfaceFlags surfaceFlags = aDecoder->GetSurfaceFlags();
 
   // Synchronously notify if we can.
-  if (NS_IsMainThread() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) {
+  if (IsOnEventTarget() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) {
     aImage->NotifyProgress(progress, invalidRect, frameCount,
                            decoderFlags, surfaceFlags);
     return;
   }
 
   // We're forced to notify asynchronously.
   NotNull<RefPtr<RasterImage>> image = aImage;
-  NS_DispatchToMainThread(NS_NewRunnableFunction(
-                            "IDecodingTask::NotifyProgress",
-                            [=]() -> void {
+  mEventTarget->Dispatch(NS_NewRunnableFunction(
+                           "IDecodingTask::NotifyProgress",
+                           [=]() -> void {
     image->NotifyProgress(progress, invalidRect, frameCount,
                           decoderFlags, surfaceFlags);
-  }));
+  }), NS_DISPATCH_NORMAL);
 }
 
-/* static */ void
+void
 IDecodingTask::NotifyDecodeComplete(NotNull<RasterImage*> aImage,
                                     NotNull<Decoder*> aDecoder)
 {
   MOZ_ASSERT(aDecoder->HasError() || !aDecoder->InFrame(),
              "Decode complete in the middle of a frame?");
+  EnsureHasEventTarget(aImage);
 
   // Capture the decoder's state.
   DecoderFinalStatus finalStatus = aDecoder->FinalStatus();
   ImageMetadata metadata = aDecoder->GetImageMetadata();
   DecoderTelemetry telemetry = aDecoder->Telemetry();
   Progress progress = aDecoder->TakeProgress();
   IntRect invalidRect = aDecoder->TakeInvalidRect();
   Maybe<uint32_t> frameCount = aDecoder->TakeCompleteFrameCount();
   DecoderFlags decoderFlags = aDecoder->GetDecoderFlags();
   SurfaceFlags surfaceFlags = aDecoder->GetSurfaceFlags();
 
   // Synchronously notify if we can.
-  if (NS_IsMainThread() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) {
+  if (IsOnEventTarget() && !(decoderFlags & DecoderFlags::ASYNC_NOTIFY)) {
     aImage->NotifyDecodeComplete(finalStatus, metadata, telemetry, progress,
                                  invalidRect, frameCount, decoderFlags,
                                  surfaceFlags);
     return;
   }
 
   // We're forced to notify asynchronously.
   NotNull<RefPtr<RasterImage>> image = aImage;
-  NS_DispatchToMainThread(NS_NewRunnableFunction(
-                            "IDecodingTask::NotifyDecodeComplete",
-                            [=]() -> void {
+  mEventTarget->Dispatch(NS_NewRunnableFunction(
+                           "IDecodingTask::NotifyDecodeComplete",
+                           [=]() -> void {
     image->NotifyDecodeComplete(finalStatus, metadata, telemetry, progress,
                                 invalidRect, frameCount, decoderFlags,
                                 surfaceFlags);
-  }));
+  }), NS_DISPATCH_NORMAL);
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // IDecodingTask implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
 void
--- a/image/IDecodingTask.h
+++ b/image/IDecodingTask.h
@@ -45,25 +45,32 @@ public:
   /// @return a priority hint that DecodePool can use when scheduling this task.
   virtual TaskPriority Priority() const = 0;
 
   /// A default implementation of IResumable which resubmits the task to the
   /// DecodePool. Subclasses can override this if they need different behavior.
   void Resume() override;
 
 protected:
+  virtual ~IDecodingTask() { }
+
   /// Notify @aImage of @aDecoder's progress.
-  static void NotifyProgress(NotNull<RasterImage*> aImage,
-                             NotNull<Decoder*> aDecoder);
+  void NotifyProgress(NotNull<RasterImage*> aImage,
+                      NotNull<Decoder*> aDecoder);
 
   /// Notify @aImage that @aDecoder has finished.
-  static void NotifyDecodeComplete(NotNull<RasterImage*> aImage,
-                                   NotNull<Decoder*> aDecoder);
+  void NotifyDecodeComplete(NotNull<RasterImage*> aImage,
+                            NotNull<Decoder*> aDecoder);
 
-  virtual ~IDecodingTask() { }
+private:
+  void EnsureHasEventTarget(NotNull<RasterImage*> aImage);
+
+  bool IsOnEventTarget() const;
+
+  nsCOMPtr<nsIEventTarget> mEventTarget;
 };
 
 
 /**
  * An IDecodingTask implementation for metadata decodes of images.
  */
 class MetadataDecodingTask final : public IDecodingTask
 {