Bug 1359833 - Part 3a. imgRequestProxy should use an event target derived from the loading document. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 19 Jul 2017 14:15:11 -0400
changeset 418435 ca9a48e8a4fbfa5b236c7da73d20d626638f91ba
parent 418434 8941aa986abc5d3ba40d8f48d840da5bbf840df1
child 418436 e347c123dad6b1694aa967643629905293bab5c5
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [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 3a. imgRequestProxy should use an event target derived from the loading document. r=tnikkel
image/imgRequestProxy.cpp
image/imgRequestProxy.h
--- a/image/imgRequestProxy.cpp
+++ b/image/imgRequestProxy.cpp
@@ -8,16 +8,18 @@
 #include "imgRequestProxy.h"
 #include "imgIOnloadBlocker.h"
 #include "imgLoader.h"
 #include "Image.h"
 #include "ImageOps.h"
 #include "nsError.h"
 #include "nsCRTGlue.h"
 #include "imgINotificationObserver.h"
+#include "mozilla/dom/TabGroup.h"       // for TabGroup
+#include "mozilla/dom/DocGroup.h"       // for DocGroup
 
 using namespace mozilla::image;
 
 // The split of imgRequestProxy and imgRequestProxyStatic means that
 // certain overridden functions need to be usable in the destructor.
 // Since virtual functions can't be used in that way, this class
 // provides a behavioural trait for each class to use instead.
 class ProxyBehaviour
@@ -149,43 +151,42 @@ imgRequestProxy::~imgRequestProxy()
     mCanceled = true;
     GetOwner()->RemoveProxy(this, NS_OK);
   }
 }
 
 nsresult
 imgRequestProxy::Init(imgRequest* aOwner,
                       nsILoadGroup* aLoadGroup,
+                      nsIDocument* aLoadingDocument,
                       ImageURL* aURI,
                       imgINotificationObserver* aObserver)
 {
   NS_PRECONDITION(!GetOwner() && !mListener,
                   "imgRequestProxy is already initialized");
 
   LOG_SCOPE_WITH_PARAM(gImgLog, "imgRequestProxy::Init", "request",
                        aOwner);
 
   MOZ_ASSERT(mAnimationConsumers == 0, "Cannot have animation before Init");
 
   mBehaviour->SetOwner(aOwner);
   mListener = aObserver;
-  // Make sure to addref mListener before the AddProxy call below, since
+  // Make sure to addref mListener before the AddToOwner call below, since
   // that call might well want to release it if the imgRequest has
   // already seen OnStopRequest.
   if (mListener) {
     mListenerIsStrongRef = true;
     NS_ADDREF(mListener);
   }
   mLoadGroup = aLoadGroup;
   mURI = aURI;
 
-  // Note: AddProxy won't send all the On* notifications immediately
-  if (GetOwner()) {
-    GetOwner()->AddProxy(this);
-  }
+  // Note: AddToOwner won't send all the On* notifications immediately
+  AddToOwner(aLoadingDocument);
 
   return NS_OK;
 }
 
 nsresult
 imgRequestProxy::ChangeOwner(imgRequest* aNewOwner)
 {
   NS_PRECONDITION(GetOwner(),
@@ -219,27 +220,106 @@ imgRequestProxy::ChangeOwner(imgRequest*
 
   // If we had animation requests, restore them here. Note that we
   // do this *after* RemoveProxy, which clears out animation consumers
   // (see bug 601723).
   for (uint32_t i = 0; i < oldAnimationConsumers; i++) {
     IncrementAnimationConsumers();
   }
 
-  GetOwner()->AddProxy(this);
+  AddToOwner(nullptr);
 
   // If we'd previously requested a synchronous decode, request a decode on the
   // new image.
   if (mDecodeRequested) {
     StartDecoding(imgIContainer::FLAG_NONE);
   }
 
   return NS_OK;
 }
 
+bool
+imgRequestProxy::IsOnEventTarget() const
+{
+  // Ensure we are in some main thread context because the scheduler group
+  // methods are only safe to call on the main thread.
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (mTabGroup) {
+    MOZ_ASSERT(mEventTarget);
+    return mTabGroup->IsSafeToRun();
+  }
+
+  if (mListener) {
+    // If we have no scheduler group but we do have a listener, then we know
+    // that the listener requires unlabelled dispatch.
+    MOZ_ASSERT(mEventTarget);
+    return mozilla::SchedulerGroup::IsSafeToRunUnlabeled();
+  }
+
+  // No listener means it is always safe, as there is nothing to do.
+  return true;
+}
+
+already_AddRefed<nsIEventTarget>
+imgRequestProxy::GetEventTarget() const
+{
+  nsCOMPtr<nsIEventTarget> target(mEventTarget);
+  return target.forget();
+}
+
+void
+imgRequestProxy::Dispatch(already_AddRefed<nsIRunnable> aEvent)
+{
+  LOG_FUNC(gImgLog, "imgRequestProxy::Dispatch");
+
+  MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mEventTarget);
+
+  mEventTarget->Dispatch(Move(aEvent), NS_DISPATCH_NORMAL);
+}
+
+void
+imgRequestProxy::AddToOwner(nsIDocument* aLoadingDocument)
+{
+  // An imgRequestProxy can be initialized with neither a listener nor a
+  // document. The caller could follow up later by cloning the canonical
+  // imgRequestProxy with the actual listener. This is possible because
+  // imgLoader::LoadImage does not require a valid listener to be provided.
+  //
+  // Without a listener, we don't need to set our scheduler group, because
+  // we have nothing to signal. However if we were told what document this
+  // is for, it is likely that future listeners will belong to the same
+  // scheduler group.
+  //
+  // With a listener, we always need to update our scheduler group. A null
+  // scheduler group is valid with or without a document, but that means
+  // we will use the most generic event target possible on dispatch.
+  if (aLoadingDocument) {
+    RefPtr<dom::DocGroup> docGroup = aLoadingDocument->GetDocGroup();
+    if (docGroup) {
+      mTabGroup = docGroup->GetTabGroup();
+      MOZ_ASSERT(mTabGroup);
+
+      mEventTarget = mTabGroup->EventTargetFor(mozilla::TaskCategory::Other);
+      MOZ_ASSERT(mEventTarget);
+    }
+  }
+
+  if (mListener && !mEventTarget) {
+    mEventTarget = do_GetMainThread();
+  }
+
+  if (!GetOwner()) {
+    return;
+  }
+
+  GetOwner()->AddProxy(this);
+}
+
 void
 imgRequestProxy::AddToLoadGroup()
 {
   NS_ASSERTION(!mIsInLoadGroup, "Whaa, we're already in the loadgroup!");
 
   if (!mIsInLoadGroup && mLoadGroup) {
     mLoadGroup->AddRequest(this, nullptr);
     mIsInLoadGroup = true;
@@ -622,29 +702,31 @@ imgRequestProxy* NewStaticProxy(imgReque
 }
 
 NS_IMETHODIMP
 imgRequestProxy::Clone(imgINotificationObserver* aObserver,
                        imgIRequest** aClone)
 {
   nsresult result;
   imgRequestProxy* proxy;
-  result = Clone(aObserver, &proxy);
+  result = Clone(aObserver, nullptr, &proxy);
   *aClone = proxy;
   return result;
 }
 
 nsresult imgRequestProxy::Clone(imgINotificationObserver* aObserver,
+                                nsIDocument* aLoadingDocument,
                                 imgRequestProxy** aClone)
 {
-  return PerformClone(aObserver, NewProxy, aClone);
+  return PerformClone(aObserver, aLoadingDocument, NewProxy, aClone);
 }
 
 nsresult
 imgRequestProxy::PerformClone(imgINotificationObserver* aObserver,
+                              nsIDocument* aLoadingDocument,
                               imgRequestProxy* (aAllocFn)(imgRequestProxy*),
                               imgRequestProxy** aClone)
 {
   NS_PRECONDITION(aClone, "Null out param");
 
   LOG_SCOPE(gImgLog, "imgRequestProxy::Clone");
 
   *aClone = nullptr;
@@ -653,17 +735,17 @@ imgRequestProxy::PerformClone(imgINotifi
   // It is important to call |SetLoadFlags()| before calling |Init()| because
   // |Init()| adds the request to the loadgroup.
   // When a request is added to a loadgroup, its load flags are merged
   // with the load flags of the loadgroup.
   // XXXldb That's not true anymore.  Stuff from imgLoader adds the
   // request to the loadgroup.
   clone->SetLoadFlags(mLoadFlags);
   nsresult rv = clone->Init(mBehaviour->GetOwner(), mLoadGroup,
-                            mURI, aObserver);
+                            aLoadingDocument, mURI, aObserver);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   if (GetOwner() && GetOwner()->GetValidator()) {
     clone->SetNotificationsDeferred(true);
     GetOwner()->GetValidator()->AddProxy(clone);
   }
@@ -806,16 +888,33 @@ imgRequestProxy::Notify(int32_t aType, c
 
   LOG_FUNC_WITH_PARAM(gImgLog, "imgRequestProxy::Notify", "type",
                       NotificationTypeToString(aType));
 
   if (!mListener || mCanceled) {
     return;
   }
 
+  if (!IsOnEventTarget()) {
+    RefPtr<imgRequestProxy> self(this);
+    if (aRect) {
+      const mozilla::gfx::IntRect rect = *aRect;
+      Dispatch(NS_NewRunnableFunction("imgRequestProxy::Notify",
+                                      [self, rect, aType]() -> void {
+        self->Notify(aType, &rect);
+      }));
+    } else {
+      Dispatch(NS_NewRunnableFunction("imgRequestProxy::Notify",
+                                      [self, aType]() -> void {
+        self->Notify(aType, nullptr);
+      }));
+    }
+    return;
+  }
+
   // Make sure the listener stays alive while we notify.
   nsCOMPtr<imgINotificationObserver> listener(mListener);
 
   listener->Notify(this, aType, aRect);
 }
 
 void
 imgRequestProxy::OnLoadComplete(bool aLastPart)
@@ -825,17 +924,25 @@ imgRequestProxy::OnLoadComplete(bool aLa
     GetName(name);
     LOG_FUNC_WITH_PARAM(gImgLog, "imgRequestProxy::OnLoadComplete",
                         "name", name.get());
   }
 
   // There's all sorts of stuff here that could kill us (the OnStopRequest call
   // on the listener, the removal from the loadgroup, the release of the
   // listener, etc).  Don't let them do it.
-  nsCOMPtr<imgIRequest> kungFuDeathGrip(this);
+  RefPtr<imgRequestProxy> self(this);
+
+  if (!IsOnEventTarget()) {
+    Dispatch(NS_NewRunnableFunction("imgRequestProxy::OnLoadComplete",
+                                    [self, aLastPart]() -> void {
+      self->OnLoadComplete(aLastPart);
+    }));
+    return;
+  }
 
   if (mListener && !mCanceled) {
     // Hold a ref to the listener while we call it, just in case.
     nsCOMPtr<imgINotificationObserver> listener(mListener);
     listener->Notify(this, imgINotificationObserver::LOAD_COMPLETE, nullptr);
   }
 
   // If we're expecting more data from a multipart channel, re-add ourself
@@ -869,35 +976,57 @@ imgRequestProxy::BlockOnload()
   if (MOZ_LOG_TEST(gImgLog, LogLevel::Debug)) {
     nsAutoCString name;
     GetName(name);
     LOG_FUNC_WITH_PARAM(gImgLog, "imgRequestProxy::BlockOnload",
                         "name", name.get());
   }
 
   nsCOMPtr<imgIOnloadBlocker> blocker = do_QueryInterface(mListener);
-  if (blocker) {
-    blocker->BlockOnload(this);
+  if (!blocker) {
+    return;
   }
+
+  if (!IsOnEventTarget()) {
+    RefPtr<imgRequestProxy> self(this);
+    Dispatch(NS_NewRunnableFunction("imgRequestProxy::BlockOnload",
+                                    [self]() -> void {
+      self->BlockOnload();
+    }));
+    return;
+  }
+
+  blocker->BlockOnload(this);
 }
 
 void
 imgRequestProxy::UnblockOnload()
 {
   if (MOZ_LOG_TEST(gImgLog, LogLevel::Debug)) {
     nsAutoCString name;
     GetName(name);
     LOG_FUNC_WITH_PARAM(gImgLog, "imgRequestProxy::UnblockOnload",
                         "name", name.get());
   }
 
   nsCOMPtr<imgIOnloadBlocker> blocker = do_QueryInterface(mListener);
-  if (blocker) {
-    blocker->UnblockOnload(this);
+  if (!blocker) {
+    return;
   }
+
+  if (!IsOnEventTarget()) {
+    RefPtr<imgRequestProxy> self(this);
+    Dispatch(NS_NewRunnableFunction("imgRequestProxy::UnblockOnload",
+                                    [self]() -> void {
+      self->UnblockOnload();
+    }));
+    return;
+  }
+
+  blocker->UnblockOnload(this);
 }
 
 void
 imgRequestProxy::NullOutListener()
 {
   // If we have animation consumers, then they don't matter anymore
   if (mListener) {
     ClearAnimationConsumers();
@@ -906,29 +1035,35 @@ imgRequestProxy::NullOutListener()
   if (mListenerIsStrongRef) {
     // Releasing could do weird reentery stuff, so just play it super-safe
     nsCOMPtr<imgINotificationObserver> obs;
     obs.swap(mListener);
     mListenerIsStrongRef = false;
   } else {
     mListener = nullptr;
   }
+
+  // Note that we don't free the event target. We actually need that to ensure
+  // we get removed from the ProgressTracker properly. No harm in keeping it
+  // however.
+  mTabGroup = nullptr;
 }
 
 NS_IMETHODIMP
 imgRequestProxy::GetStaticRequest(imgIRequest** aReturn)
 {
   imgRequestProxy* proxy;
-  nsresult result = GetStaticRequest(&proxy);
+  nsresult result = GetStaticRequest(nullptr, &proxy);
   *aReturn = proxy;
   return result;
 }
 
 nsresult
-imgRequestProxy::GetStaticRequest(imgRequestProxy** aReturn)
+imgRequestProxy::GetStaticRequest(nsIDocument* aLoadingDocument,
+                                  imgRequestProxy** aReturn)
 {
   *aReturn = nullptr;
   RefPtr<Image> image = GetImage();
 
   bool animated;
   if (!image || (NS_SUCCEEDED(image->GetAnimated(&animated)) && !animated)) {
     // Early exit - we're not animated, so we don't have to do anything.
     NS_ADDREF(*aReturn = this);
@@ -945,17 +1080,17 @@ imgRequestProxy::GetStaticRequest(imgReq
   // We are animated. We need to create a frozen version of this image.
   RefPtr<Image> frozenImage = ImageOps::Freeze(image);
 
   // Create a static imgRequestProxy with our new extracted frame.
   nsCOMPtr<nsIPrincipal> currentPrincipal;
   GetImagePrincipal(getter_AddRefs(currentPrincipal));
   RefPtr<imgRequestProxy> req = new imgRequestProxyStatic(frozenImage,
                                                             currentPrincipal);
-  req->Init(nullptr, nullptr, mURI, nullptr);
+  req->Init(nullptr, nullptr, aLoadingDocument, mURI, nullptr);
 
   NS_ADDREF(*aReturn = req);
 
   return NS_OK;
 }
 
 void
 imgRequestProxy::NotifyListener()
@@ -1100,12 +1235,13 @@ imgRequestProxyStatic::GetImagePrincipal
 
   NS_ADDREF(*aPrincipal = mPrincipal);
 
   return NS_OK;
 }
 
 nsresult
 imgRequestProxyStatic::Clone(imgINotificationObserver* aObserver,
+                             nsIDocument* aLoadingDocument,
                              imgRequestProxy** aClone)
 {
-  return PerformClone(aObserver, NewStaticProxy, aClone);
+  return PerformClone(aObserver, aLoadingDocument, NewStaticProxy, aClone);
 }
--- a/image/imgRequestProxy.h
+++ b/image/imgRequestProxy.h
@@ -30,16 +30,20 @@
     {0x8f, 0x65, 0x9c, 0x46, 0x2e, 0xe2, 0xbc, 0x95} \
 }
 
 class imgINotificationObserver;
 class imgStatusNotifyRunnable;
 class ProxyBehaviour;
 
 namespace mozilla {
+namespace dom {
+class TabGroup;
+}
+
 namespace image {
 class Image;
 class ImageURL;
 class ProgressTracker;
 } // namespace image
 } // namespace mozilla
 
 class imgRequestProxy : public imgIRequest,
@@ -65,16 +69,17 @@ public:
   // nsITimedChannel declared below
 
   imgRequestProxy();
 
   // Callers to Init or ChangeOwner are required to call NotifyListener after
   // (although not immediately after) doing so.
   nsresult Init(imgRequest* aOwner,
                 nsILoadGroup* aLoadGroup,
+                nsIDocument* aLoadingDocument,
                 ImageURL* aURI,
                 imgINotificationObserver* aObserver);
 
   nsresult ChangeOwner(imgRequest* aNewOwner); // this will change mOwner.
                                                // Do not call this if the
                                                // previous owner has already
                                                // sent notifications out!
 
@@ -114,25 +119,30 @@ public:
   {
     return mDeferNotifications;
   }
   virtual void SetNotificationsDeferred(bool aDeferNotifications) override
   {
     mDeferNotifications = aDeferNotifications;
   }
 
+  bool IsOnEventTarget() const;
+  already_AddRefed<nsIEventTarget> GetEventTarget() const override;
+
   // Removes all animation consumers that were created with
   // IncrementAnimationConsumers. This is necessary since we need
   // to do it before the proxy itself is destroyed. See
   // imgRequest::RemoveProxy
   void ClearAnimationConsumers();
 
   virtual nsresult Clone(imgINotificationObserver* aObserver,
+                         nsIDocument* aLoadingDocument,
                          imgRequestProxy** aClone);
-  nsresult GetStaticRequest(imgRequestProxy** aReturn);
+  nsresult GetStaticRequest(nsIDocument* aLoadingDocument,
+                            imgRequestProxy** aReturn);
 
   nsresult GetURI(ImageURL** aURI);
 
 protected:
   friend class mozilla::image::ProgressTracker;
   friend class imgStatusNotifyRunnable;
 
   class imgCancelRunnable;
@@ -179,40 +189,47 @@ protected:
     return GetOwner()->GetTimedChannel();
   }
 
   already_AddRefed<Image> GetImage() const;
   bool HasImage() const;
   imgRequest* GetOwner() const;
 
   nsresult PerformClone(imgINotificationObserver* aObserver,
+                        nsIDocument* aLoadingDocument,
                         imgRequestProxy* (aAllocFn)(imgRequestProxy*),
                         imgRequestProxy** aClone);
 
 public:
   NS_FORWARD_SAFE_NSITIMEDCHANNEL(TimedChannel())
 
 protected:
   mozilla::UniquePtr<ProxyBehaviour> mBehaviour;
 
 private:
   friend class imgCacheValidator;
   friend imgRequestProxy* NewStaticProxy(imgRequestProxy* aThis);
 
+  void AddToOwner(nsIDocument* aLoadingDocument);
+
+  void Dispatch(already_AddRefed<nsIRunnable> aEvent);
+
   // The URI of our request.
   RefPtr<ImageURL> mURI;
 
   // mListener is only promised to be a weak ref (see imgILoader.idl),
   // but we actually keep a strong ref to it until we've seen our
   // first OnStopRequest.
   imgINotificationObserver* MOZ_UNSAFE_REF("Observers must call Cancel() or "
                                            "CancelAndForgetObserver() before "
                                            "they are destroyed") mListener;
 
   nsCOMPtr<nsILoadGroup> mLoadGroup;
+  RefPtr<mozilla::dom::TabGroup> mTabGroup;
+  nsCOMPtr<nsIEventTarget> mEventTarget;
 
   nsLoadFlags mLoadFlags;
   uint32_t    mLockCount;
   uint32_t    mAnimationConsumers;
   bool mCanceled;
   bool mIsInLoadGroup;
   bool mListenerIsStrongRef;
   bool mDecodeRequested;
@@ -230,16 +247,17 @@ class imgRequestProxyStatic : public img
 public:
   imgRequestProxyStatic(Image* aImage, nsIPrincipal* aPrincipal);
 
   NS_IMETHOD GetImagePrincipal(nsIPrincipal** aPrincipal) override;
 
   using imgRequestProxy::Clone;
 
   virtual nsresult Clone(imgINotificationObserver* aObserver,
+                         nsIDocument* aLoadingDocument,
                          imgRequestProxy** aClone) override;
 
 protected:
   friend imgRequestProxy* NewStaticProxy(imgRequestProxy*);
 
   // Our principal. We have to cache it, rather than accessing the underlying
   // request on-demand, because static proxies don't have an underlying request.
   nsCOMPtr<nsIPrincipal> mPrincipal;