Bug 1359833 - Part 6. nsImageLoadingContent should not associate scripted and XPCOM observers with the same document. r=tnikkel

2017-07-19 14:15:11 -04:00 · 2017-07-19 14:15:11 -04:00 · a0321e7db6
--- a/dom/base/nsIImageLoadingContent.idl
+++ b/dom/base/nsIImageLoadingContent.idl
@ -81,13 +81,25 @@ interface nsIImageLoadingContent : imgINotificationObserver
   *
   * @throws NS_ERROR_OUT_OF_MEMORY
   */
-  void addObserver(in imgINotificationObserver aObserver);
+  [notxpcom] nsresult addNativeObserver(in imgINotificationObserver aObserver);

  /**
   * Used to unregister an image decoder observer.
   *
   * @param aObserver the observer to unregister
   */
+  [notxpcom] nsresult removeNativeObserver(in imgINotificationObserver aObserver);
+
+  /**
+   * Same as addNativeObserver but intended for scripted observers or observers
+   * from another or without a document.
+   */
+  void addObserver(in imgINotificationObserver aObserver);
+
+  /**
+   * Same as removeNativeObserver but intended for scripted observers or
+   * observers from another or without a document.
+   */
  void removeObserver(in imgINotificationObserver aObserver);
  
  /**
--- a/dom/base/nsImageLoadingContent.cpp
+++ b/dom/base/nsImageLoadingContent.cpp
@ -123,6 +123,8 @@ nsImageLoadingContent::~nsImageLoadingContent()
               "DestroyImageLoadingContent not called");
  NS_ASSERTION(!mObserverList.mObserver && !mObserverList.mNext,
               "Observers still registered?");
+  NS_ASSERTION(mScriptedObservers.IsEmpty(),
+               "Scripted observers still registered?");
 }

 /*
@ -387,7 +389,7 @@ ReplayImageStatus(imgIRequest* aRequest, imgINotificationObserver* aObserver)
 }

 NS_IMETHODIMP
-nsImageLoadingContent::AddObserver(imgINotificationObserver* aObserver)
+nsImageLoadingContent::AddNativeObserver(imgINotificationObserver* aObserver)
 {
  NS_ENSURE_ARG_POINTER(aObserver);

@ -416,7 +418,7 @@ nsImageLoadingContent::AddObserver(imgINotificationObserver* aObserver)
 }

 NS_IMETHODIMP
-nsImageLoadingContent::RemoveObserver(imgINotificationObserver* aObserver)
+nsImageLoadingContent::RemoveNativeObserver(imgINotificationObserver* aObserver)
 {
  NS_ENSURE_ARG_POINTER(aObserver);

@ -449,6 +451,157 @@ nsImageLoadingContent::RemoveObserver(imgINotificationObserver* aObserver)
  return NS_OK;
 }

+NS_IMETHODIMP
+nsImageLoadingContent::AddObserver(imgINotificationObserver* aObserver)
+{
+  NS_ENSURE_ARG_POINTER(aObserver);
+
+  nsresult rv = NS_OK;
+  RefPtr<imgRequestProxy> currentReq;
+  if (mCurrentRequest) {
+    // Scripted observers may not belong to the same document as us, so when we
+    // create the imgRequestProxy, we shouldn't use any. This allows the request
+    // to dispatch notifications from the correct scheduler group.
+    rv = mCurrentRequest->Clone(aObserver, nullptr, getter_AddRefs(currentReq));
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+  }
+
+  RefPtr<imgRequestProxy> pendingReq;
+  if (mPendingRequest) {
+    // See above for why we don't use the loading document.
+    rv = mPendingRequest->Clone(aObserver, nullptr, getter_AddRefs(pendingReq));
+    if (NS_FAILED(rv)) {
+      mCurrentRequest->CancelAndForgetObserver(NS_BINDING_ABORTED);
+      return rv;
+    }
+  }
+
+  mScriptedObservers.AppendElement(
+    new ScriptedImageObserver(aObserver, Move(currentReq), Move(pendingReq)));
+  return rv;
+}
+
+NS_IMETHODIMP
+nsImageLoadingContent::RemoveObserver(imgINotificationObserver* aObserver)
+{
+  NS_ENSURE_ARG_POINTER(aObserver);
+
+  if (NS_WARN_IF(mScriptedObservers.IsEmpty())) {
+    return NS_OK;
+  }
+
+  RefPtr<ScriptedImageObserver> observer;
+  auto i = mScriptedObservers.Length();
+  do {
+    --i;
+    if (mScriptedObservers[i]->mObserver == aObserver) {
+      observer = Move(mScriptedObservers[i]);
+      mScriptedObservers.RemoveElementAt(i);
+      break;
+    }
+  } while(i > 0);
+
+  if (NS_WARN_IF(!observer)) {
+    return NS_OK;
+  }
+
+  // If the cancel causes a mutation, it will be harmless, because we have
+  // already removed the observer from the list.
+  observer->CancelRequests();
+  return NS_OK;
+}
+
+void
+nsImageLoadingContent::ClearScriptedRequests(int32_t aRequestType, nsresult aReason)
+{
+  if (MOZ_LIKELY(mScriptedObservers.IsEmpty())) {
+    return;
+  }
+
+  nsTArray<RefPtr<ScriptedImageObserver>> observers(mScriptedObservers);
+  auto i = observers.Length();
+  do {
+    --i;
+
+    RefPtr<imgRequestProxy> req;
+    switch (aRequestType) {
+    case CURRENT_REQUEST:
+      req = Move(observers[i]->mCurrentRequest);
+      break;
+    case PENDING_REQUEST:
+      req = Move(observers[i]->mPendingRequest);
+      break;
+    default:
+      NS_ERROR("Unknown request type");
+      return;
+    }
+
+    if (req) {
+      req->CancelAndForgetObserver(aReason);
+    }
+  } while (i > 0);
+}
+
+void
+nsImageLoadingContent::CloneScriptedRequests(imgRequestProxy* aRequest)
+{
+  MOZ_ASSERT(aRequest);
+
+  if (MOZ_LIKELY(mScriptedObservers.IsEmpty())) {
+    return;
+  }
+
+  bool current;
+  if (aRequest == mCurrentRequest) {
+    current = true;
+  } else if (aRequest == mPendingRequest) {
+    current = false;
+  } else {
+    MOZ_ASSERT_UNREACHABLE("Unknown request type");
+    return;
+  }
+
+  nsTArray<RefPtr<ScriptedImageObserver>> observers(mScriptedObservers);
+  auto i = observers.Length();
+  do {
+    --i;
+
+    ScriptedImageObserver* observer = observers[i];
+    RefPtr<imgRequestProxy>& req = current ? observer->mCurrentRequest
+                                           : observer->mPendingRequest;
+    if (NS_WARN_IF(req)) {
+      MOZ_ASSERT_UNREACHABLE("Should have cancelled original request");
+      req->CancelAndForgetObserver(NS_BINDING_ABORTED);
+      req = nullptr;
+    }
+
+    nsresult rv = aRequest->Clone(observer->mObserver, nullptr, getter_AddRefs(req));
+    Unused << NS_WARN_IF(NS_FAILED(rv));
+  } while (i > 0);
+}
+
+void
+nsImageLoadingContent::MakePendingScriptedRequestsCurrent()
+{
+  if (MOZ_LIKELY(mScriptedObservers.IsEmpty())) {
+    return;
+  }
+
+  nsTArray<RefPtr<ScriptedImageObserver>> observers(mScriptedObservers);
+  auto i = observers.Length();
+  do {
+    --i;
+
+    ScriptedImageObserver* observer = observers[i];
+    if (observer->mCurrentRequest) {
+      observer->mCurrentRequest->CancelAndForgetObserver(NS_BINDING_ABORTED);
+    }
+    observer->mCurrentRequest = Move(observer->mPendingRequest);
+  } while (i > 0);
+}
+
 already_AddRefed<imgIRequest>
 nsImageLoadingContent::GetRequest(int32_t aRequestType,
                                  ErrorResult& aError)
@ -633,6 +786,7 @@ nsImageLoadingContent::LoadImageWithChannel(nsIChannel* aChannel,
  nsresult rv = loader->
    LoadImageWithChannel(aChannel, this, doc, aListener, getter_AddRefs(req));
  if (NS_SUCCEEDED(rv)) {
+    CloneScriptedRequests(req);
    TrackImage(req);
    ResetAnimationIfNeeded();
    return NS_OK;
@ -910,6 +1064,7 @@ nsImageLoadingContent::LoadImage(nsIURI* aNewURI,
  aDocument->ForgetImagePreload(aNewURI);

  if (NS_SUCCEEDED(rv)) {
+    CloneScriptedRequests(req);
    TrackImage(req);
    ResetAnimationIfNeeded();

@ -1085,6 +1240,7 @@ nsImageLoadingContent::UseAsPrimaryRequest(imgRequestProxy* aRequest,
  RefPtr<imgRequestProxy>& req = PrepareNextRequest(aImageLoadType);
  nsresult rv = aRequest->SyncClone(this, GetOurOwnerDoc(), getter_AddRefs(req));
  if (NS_SUCCEEDED(rv)) {
+    CloneScriptedRequests(req);
    TrackImage(req);
  } else {
    MOZ_ASSERT(!req, "Shouldn't have non-null request here");
@ -1334,6 +1490,7 @@ nsImageLoadingContent::MakePendingRequestCurrent()
                                                 : eImageLoadType_Normal;

  PrepareCurrentRequest(loadType) = mPendingRequest;
+  MakePendingScriptedRequestsCurrent();
  mPendingRequest = nullptr;
  mCurrentRequestFlags = mPendingRequestFlags;
  mPendingRequestFlags = 0;
@ -1363,6 +1520,7 @@ nsImageLoadingContent::ClearCurrentRequest(nsresult aReason,

  // Clean up the request.
  UntrackImage(mCurrentRequest, aNonvisibleAction);
+  ClearScriptedRequests(CURRENT_REQUEST, aReason);
  mCurrentRequest->CancelAndForgetObserver(aReason);
  mCurrentRequest = nullptr;
  mCurrentRequestFlags = 0;
@ -1381,6 +1539,7 @@ nsImageLoadingContent::ClearPendingRequest(nsresult aReason,
                                        &mPendingRequestRegistered);

  UntrackImage(mPendingRequest, aNonvisibleAction);
+  ClearScriptedRequests(PENDING_REQUEST, aReason);
  mPendingRequest->CancelAndForgetObserver(aReason);
  mPendingRequest = nullptr;
  mPendingRequestFlags = 0;
@ -1568,8 +1727,12 @@ nsImageLoadingContent::UntrackImage(imgIRequest* aImage,
 void
 nsImageLoadingContent::CreateStaticImageClone(nsImageLoadingContent* aDest) const
 {
+  aDest->ClearScriptedRequests(CURRENT_REQUEST, NS_BINDING_ABORTED);
  aDest->mCurrentRequest =
    nsContentUtils::GetStaticRequest(aDest->GetOurOwnerDoc(), mCurrentRequest);
+  if (aDest->mCurrentRequest) {
+    aDest->CloneScriptedRequests(aDest->mCurrentRequest);
+  }
  aDest->TrackImage(aDest->mCurrentRequest);
  aDest->mForcedImageState = mForcedImageState;
  aDest->mImageBlockingStatus = mImageBlockingStatus;
@ -1601,6 +1764,38 @@ nsImageLoadingContent::ImageObserver::~ImageObserver()
  NS_CONTENT_DELETE_LIST_MEMBER(ImageObserver, this, mNext);
 }

+nsImageLoadingContent::ScriptedImageObserver::ScriptedImageObserver(imgINotificationObserver* aObserver,
+                                                                    RefPtr<imgRequestProxy>&& aCurrentRequest,
+                                                                    RefPtr<imgRequestProxy>&& aPendingRequest)
+  : mObserver(aObserver)
+  , mCurrentRequest(aCurrentRequest)
+  , mPendingRequest(aPendingRequest)
+{ }
+
+nsImageLoadingContent::ScriptedImageObserver::~ScriptedImageObserver()
+{
+  // We should have cancelled any requests before getting released.
+  DebugOnly<bool> cancel = CancelRequests();
+  MOZ_ASSERT(!cancel, "Still have requests in ~ScriptedImageObserver!");
+}
+
+bool
+nsImageLoadingContent::ScriptedImageObserver::CancelRequests()
+{
+  bool cancelled = false;
+  if (mCurrentRequest) {
+    mCurrentRequest->CancelAndForgetObserver(NS_BINDING_ABORTED);
+    mCurrentRequest = nullptr;
+    cancelled = true;
+  }
+  if (mPendingRequest) {
+    mPendingRequest->CancelAndForgetObserver(NS_BINDING_ABORTED);
+    mPendingRequest = nullptr;
+    cancelled = true;
+  }
+  return cancelled;
+}
+
 // Only HTMLInputElement.h overrides this for <img> tags
 // all other subclasses use this one, i.e. ignore referrer attributes
 mozilla::net::ReferrerPolicy
--- a/dom/base/nsImageLoadingContent.h
+++ b/dom/base/nsImageLoadingContent.h
@ -225,7 +225,7 @@ protected:

 private:
  /**
-   * Struct used to manage the image observers.
+   * Struct used to manage the native image observers.
   */
  struct ImageObserver {
    explicit ImageObserver(imgINotificationObserver* aObserver);
@ -235,6 +235,26 @@ private:
    ImageObserver* mNext;
  };

+  /**
+   * Struct used to manage the scripted/XPCOM image observers.
+   */
+  class ScriptedImageObserver final {
+  public:
+    NS_INLINE_DECL_REFCOUNTING(ScriptedImageObserver)
+
+    ScriptedImageObserver(imgINotificationObserver* aObserver,
+                          RefPtr<imgRequestProxy>&& aCurrentRequest,
+                          RefPtr<imgRequestProxy>&& aPendingRequest);
+    bool CancelRequests();
+
+    nsCOMPtr<imgINotificationObserver> mObserver;
+    RefPtr<imgRequestProxy> mCurrentRequest;
+    RefPtr<imgRequestProxy> mPendingRequest;
+
+  private:
+    ~ScriptedImageObserver();
+  };
+
  /**
   * Struct to report state changes
   */
@ -402,6 +422,24 @@ protected:
  nsCOMPtr<nsIURI>      mCurrentURI;

 private:
+  /**
+   * Clones the given "current" or "pending" request for each scripted observer.
+   */
+  void CloneScriptedRequests(imgRequestProxy* aRequest);
+
+  /**
+   * Cancels and nulls-out the "current" or "pending" requests if they exist
+   * for each scripted observer.
+   */
+  void ClearScriptedRequests(int32_t aRequestType, nsresult aReason);
+
+  /**
+   * Moves the "pending" request into the "current" request for each scripted
+   * observer. If there is an existing "current" request, it will cancel it
+   * first.
+   */
+  void MakePendingScriptedRequestsCurrent();
+
  /**
   * Typically we will have only one observer (our frame in the screen
   * prescontext), so we want to only make space for one and to
@ -412,6 +450,13 @@ private:
   */
  ImageObserver mObserverList;

+  /**
+   * Typically we will have no scripted observers, as this is only used by
+   * chrome, legacy extensions, and some mochitests. An empty array reserves
+   * minimal memory.
+   */
+  nsTArray<RefPtr<ScriptedImageObserver>> mScriptedObservers;
+
  /**
   * When mIsImageStateForced is true, this holds the ImageState that we'll
   * return in ImageState().