Bug 1359833 - Part 1. ProgressTracker should select an event target from observers and expose to callers. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 19 Jul 2017 14:15:11 -0400
changeset 420835 fd25a183897f665d70086ebd97033cb0f851c58c
parent 420834 ae4ec8080af6dbffc046e8699c8246f564046362
child 420836 8941aa986abc5d3ba40d8f48d840da5bbf840df1
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [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 1. ProgressTracker should select an event target from observers and expose to callers. r=tnikkel
image/IProgressObserver.h
image/ProgressTracker.cpp
image/ProgressTracker.h
--- a/image/IProgressObserver.h
+++ b/image/IProgressObserver.h
@@ -5,16 +5,18 @@
 
 #ifndef mozilla_image_IProgressObserver_h
 #define mozilla_image_IProgressObserver_h
 
 #include "mozilla/WeakPtr.h"
 #include "nsISupports.h"
 #include "nsRect.h"
 
+class nsIEventTarget;
+
 namespace mozilla {
 namespace image {
 
 /**
  * An interface for observing changes to image state, as reported by
  * ProgressTracker.
  *
  * This is the ImageLib-internal version of imgINotificationObserver,
@@ -42,16 +44,21 @@ public:
   virtual void BlockOnload() = 0;
   virtual void UnblockOnload() = 0;
 
   // Other, internal-only methods:
   virtual void SetHasImage() = 0;
   virtual bool NotificationsDeferred() const = 0;
   virtual void SetNotificationsDeferred(bool aDeferNotifications) = 0;
 
+  virtual already_AddRefed<nsIEventTarget> GetEventTarget() const
+  {
+    return nullptr;
+  }
+
 protected:
   virtual ~IProgressObserver() { }
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_IProgressObserver_h
--- a/image/ProgressTracker.cpp
+++ b/image/ProgressTracker.cpp
@@ -11,16 +11,17 @@
 #include "imgINotificationObserver.h"
 #include "imgIRequest.h"
 #include "Image.h"
 #include "nsNetUtil.h"
 #include "nsIObserverService.h"
 
 #include "mozilla/Assertions.h"
 #include "mozilla/Services.h"
+#include "mozilla/SystemGroup.h"
 
 using mozilla::WeakPtr;
 
 namespace mozilla {
 namespace image {
 
 static void
 CheckProgressConsistency(Progress aOldProgress, Progress aNewProgress, bool aIsMultipart)
@@ -64,29 +65,39 @@ CheckProgressConsistency(Progress aOldPr
   if (aNewProgress & FLAG_LAST_PART_COMPLETE) {
     MOZ_ASSERT(aNewProgress & FLAG_LOAD_COMPLETE);
   }
   if (aNewProgress & FLAG_HAS_ERROR) {
     // No preconditions.
   }
 }
 
+ProgressTracker::ProgressTracker()
+  : mMutex("ProgressTracker::mMutex")
+  , mImage(nullptr)
+  , mEventTarget(WrapNotNull(nsCOMPtr<nsIEventTarget>(SystemGroup::EventTargetFor(TaskCategory::Other))))
+  , mObserversWithTargets(0)
+  , mObservers(new ObserverTable)
+  , mProgress(NoProgress)
+  , mIsMultipart(false)
+{ }
+
 void
 ProgressTracker::SetImage(Image* aImage)
 {
-  MutexAutoLock lock(mImageMutex);
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(aImage, "Setting null image");
   MOZ_ASSERT(!mImage, "Setting image when we already have one");
   mImage = aImage;
 }
 
 void
 ProgressTracker::ResetImage()
 {
-  MutexAutoLock lock(mImageMutex);
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mImage, "Resetting image when it's already null!");
   mImage = nullptr;
 }
 
 uint32_t
 ProgressTracker::GetImageStatus() const
 {
   uint32_t status = imgIRequest::STATUS_NONE;
@@ -188,17 +199,17 @@ ProgressTracker::Notify(IProgressObserve
   // unnecessarily delay onload.
   AsyncNotifyRunnable* runnable =
     static_cast<AsyncNotifyRunnable*>(mRunnable.get());
 
   if (runnable) {
     runnable->AddObserver(aObserver);
   } else {
     mRunnable = new AsyncNotifyRunnable(this, aObserver);
-    NS_DispatchToCurrentThread(mRunnable);
+    mEventTarget->Dispatch(mRunnable, NS_DISPATCH_NORMAL);
   }
 }
 
 // A helper class to allow us to call SyncNotify asynchronously for a given,
 // fixed, state.
 class AsyncNotifyCurrentStateRunnable : public Runnable
 {
   public:
@@ -246,17 +257,17 @@ ProgressTracker::NotifyCurrentState(IPro
     LOG_FUNC_WITH_PARAM(gImgLog,
                         "ProgressTracker::NotifyCurrentState", "uri", spec.get());
   }
 
   aObserver->SetNotificationsDeferred(true);
 
   nsCOMPtr<nsIRunnable> ev = new AsyncNotifyCurrentStateRunnable(this,
                                                                  aObserver);
-  NS_DispatchToCurrentThread(ev);
+  mEventTarget->Dispatch(ev.forget(), NS_DISPATCH_NORMAL);
 }
 
 /**
  * ImageObserverNotifier is a helper type that abstracts over the difference
  * between sending notifications to all of the observers in an ObserverTable,
  * and sending them to a single observer. This allows the same notification code
  * to be used for both cases.
  */
@@ -443,42 +454,93 @@ ProgressTracker::EmulateRequestFinished(
     aObserver->UnblockOnload();
   }
 
   if (!(mProgress & FLAG_LOAD_COMPLETE)) {
     aObserver->OnLoadComplete(true);
   }
 }
 
+already_AddRefed<nsIEventTarget>
+ProgressTracker::GetEventTarget() const
+{
+  MutexAutoLock lock(mMutex);
+  nsCOMPtr<nsIEventTarget> target = mEventTarget;
+  return target.forget();
+}
+
 void
 ProgressTracker::AddObserver(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread());
   RefPtr<IProgressObserver> observer = aObserver;
 
+  nsCOMPtr<nsIEventTarget> target = observer->GetEventTarget();
+  if (target) {
+    if (mObserversWithTargets == 0) {
+      // On the first observer with a target (i.e. listener), always accept its
+      // event target; this may be for a specific DocGroup, or it may be the
+      // unlabelled main thread target.
+      MutexAutoLock lock(mMutex);
+      mEventTarget = WrapNotNull(target);
+    } else if (mEventTarget.get() != target.get()) {
+      // If a subsequent observer comes in with a different target, we need to
+      // switch to use the unlabelled main thread target, if we haven't already.
+      MutexAutoLock lock(mMutex);
+      nsCOMPtr<nsIEventTarget> mainTarget(do_GetMainThread());
+      mEventTarget = WrapNotNull(mainTarget);
+    }
+    ++mObserversWithTargets;
+  }
+
   mObservers.Write([=](ObserverTable* aTable) {
     MOZ_ASSERT(!aTable->Get(observer, nullptr),
                "Adding duplicate entry for image observer");
 
     WeakPtr<IProgressObserver> weakPtr = observer.get();
     aTable->Put(observer, weakPtr);
   });
+
+  MOZ_ASSERT(mObserversWithTargets <= ObserverCount());
 }
 
 bool
 ProgressTracker::RemoveObserver(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread());
   RefPtr<IProgressObserver> observer = aObserver;
 
   // Remove the observer from the list.
-  bool removed = mObservers.Write([=](ObserverTable* aTable) {
+  bool removed = mObservers.Write([observer](ObserverTable* aTable) {
     return aTable->Remove(observer);
   });
 
+  // Sometimes once an image is decoded, and all of its observers removed, a new
+  // document may request the same image. Thus we need to clear our event target
+  // state when the last observer is removed, so that we select the most
+  // appropriate event target when a new observer is added. Since the event
+  // target may have changed (e.g. due to the scheduler group going away before
+  // we were removed), so we should be cautious comparing this target against
+  // anything at this stage.
+  if (removed) {
+    nsCOMPtr<nsIEventTarget> target = observer->GetEventTarget();
+    if (target) {
+      MOZ_ASSERT(mObserversWithTargets > 0);
+      --mObserversWithTargets;
+
+      if (mObserversWithTargets == 0) {
+        MutexAutoLock lock(mMutex);
+        nsCOMPtr<nsIEventTarget> target(SystemGroup::EventTargetFor(TaskCategory::Other));
+        mEventTarget = WrapNotNull(target);
+      }
+    }
+
+    MOZ_ASSERT(mObserversWithTargets <= ObserverCount());
+  }
+
   // Observers can get confused if they don't get all the proper teardown
   // notifications. Part ways on good terms.
   if (removed && !aObserver->NotificationsDeferred()) {
     EmulateRequestFinished(aObserver);
   }
 
   // Make sure we don't give callbacks to an observer that isn't interested in
   // them any more.
--- a/image/ProgressTracker.h
+++ b/image/ProgressTracker.h
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_image_ProgressTracker_h
 #define mozilla_image_ProgressTracker_h
 
 #include "CopyOnWrite.h"
+#include "mozilla/NotNull.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/WeakPtr.h"
 #include "nsDataHashtable.h"
 #include "nsCOMPtr.h"
 #include "nsTObserverArray.h"
 #include "nsThreadUtils.h"
 #include "nsRect.h"
@@ -108,28 +109,22 @@ private:
 class ProgressTracker : public mozilla::SupportsWeakPtr<ProgressTracker>
 {
   virtual ~ProgressTracker() { }
 
 public:
   MOZ_DECLARE_WEAKREFERENCE_TYPENAME(ProgressTracker)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ProgressTracker)
 
-  ProgressTracker()
-    : mImageMutex("ProgressTracker::mImage")
-    , mImage(nullptr)
-    , mObservers(new ObserverTable)
-    , mProgress(NoProgress)
-    , mIsMultipart(false)
-  { }
+  ProgressTracker();
 
-  bool HasImage() const { MutexAutoLock lock(mImageMutex); return mImage; }
+  bool HasImage() const { MutexAutoLock lock(mMutex); return mImage; }
   already_AddRefed<Image> GetImage() const
   {
-    MutexAutoLock lock(mImageMutex);
+    MutexAutoLock lock(mMutex);
     RefPtr<Image> image = mImage;
     return image.forget();
   }
 
   // Get the current image status (as in imgIRequest).
   uint32_t GetImageStatus() const;
 
   // Get the current Progress.
@@ -187,16 +182,19 @@ public:
                           const nsIntRect& aInvalidRect = nsIntRect());
 
   // We manage a set of observers that are using an image and thus concerned
   // with its loading progress. Weak pointers.
   void AddObserver(IProgressObserver* aObserver);
   bool RemoveObserver(IProgressObserver* aObserver);
   uint32_t ObserverCount() const;
 
+  // Get the event target we should currently dispatch events to.
+  already_AddRefed<nsIEventTarget> GetEventTarget() const;
+
   // Resets our weak reference to our image. Image subclasses should call this
   // in their destructor.
   void ResetImage();
 
   // Tell this progress tracker that it is for a multipart image.
   void SetIsMultipart() { mIsMultipart = true; }
 
 private:
@@ -215,21 +213,39 @@ private:
   void EmulateRequestFinished(IProgressObserver* aObserver);
 
   // Main thread only because it deals with the observer service.
   void FireFailureNotification();
 
   // The runnable, if any, that we've scheduled to deliver async notifications.
   nsCOMPtr<nsIRunnable> mRunnable;
 
+  // mMutex protects access to mImage and mEventTarget.
+  mutable Mutex mMutex;
+
   // mImage is a weak ref; it should be set to null when the image goes out of
-  // scope. mImageMutex protects mImage.
-  mutable Mutex mImageMutex;
+  // scope.
   Image* mImage;
 
+  // mEventTarget is the current, best effort event target to dispatch
+  // notifications to from the decoder threads. It will change as observers are
+  // added and removed (see mObserversWithTargets).
+  NotNull<nsCOMPtr<nsIEventTarget>> mEventTarget;
+
+  // How many observers have been added that have an explicit event target.
+  // When the first observer is added with an explicit event target, we will
+  // default to that as long as all observers use the same target. If a new
+  // observer is added which has a different event target, we will switch to
+  // using the unlabeled main thread event target which is safe for all
+  // observers. If all observers with explicit event targets are removed, we
+  // will revert back to the initial event target (for SystemGroup). An
+  // observer without an explicit event target does not care what context it
+  // is dispatched in, and thus does not impact the state.
+  uint32_t mObserversWithTargets;
+
   // Hashtable of observers attached to the image. Each observer represents a
   // consumer using the image. Main thread only.
   CopyOnWrite<ObserverTable> mObservers;
 
   Progress mProgress;
 
   // Whether this is a progress tracker for a multipart image.
   bool mIsMultipart;