Bug 1338446 Part 4 - Label StyleImageRequestCleanupTask. draft
authorTing-Yu Lin <tlin@mozilla.com>
Tue, 14 Mar 2017 21:29:55 +0800
changeset 501449 62879f361491d0f9714c47262f651b7a3ccd406e
parent 501448 d938012ea8d1651104e1c5e5459cf892b9c061f9
child 501450 95994f0907e20d7dfbdf4aa92809f27a92d07d29
push id49988
push userbmo:tlin@mozilla.com
push dateMon, 20 Mar 2017 10:12:33 +0000
bugs1338446
milestone55.0a1
Bug 1338446 Part 4 - Label StyleImageRequestCleanupTask. If nsStyleImageRequest::Resolve() has been called, we cache the DocGroup and use it for dispatching events for the clean up task. Otherwise, it's safe to do clean up task on non-main thread. MozReview-Commit-ID: BXalEkc6dBm
layout/style/nsCSSValue.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -3052,17 +3052,19 @@ css::ImageValue::Initialize(nsIDocument*
 
 #ifdef DEBUG
   mInitialized = true;
 #endif
 }
 
 css::ImageValue::~ImageValue()
 {
-  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(NS_IsMainThread() || mRequests.Count() == 0,
+             "Destructor should run on main thread, or on non-main thread "
+             "when mRequest is empty!");
 
   for (auto iter = mRequests.Iter(); !iter.Done(); iter.Next()) {
     nsIDocument* doc = iter.Key();
     RefPtr<imgRequestProxy>& proxy = iter.Data();
 
     if (doc) {
       doc->StyleImageLoader()->DeregisterCSSImage(this);
     }
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -28,16 +28,17 @@
 #include "nsBidiUtils.h"
 #include "nsLayoutUtils.h"
 
 #include "imgIRequest.h"
 #include "imgIContainer.h"
 #include "CounterStyleManager.h"
 
 #include "mozilla/dom/AnimationEffectReadOnlyBinding.h" // for PlaybackDirection
+#include "mozilla/dom/DocGroup.h"
 #include "mozilla/dom/ImageTracker.h"
 #include "mozilla/Likely.h"
 #include "nsIURI.h"
 #include "nsIDocument.h"
 #include <algorithm>
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -1885,17 +1886,17 @@ nsStyleGradient::HasCalc()
 }
 
 
 // --------------------
 // nsStyleImageRequest
 
 /**
  * Runnable to release the nsStyleImageRequest's mRequestProxy,
- * mImageValue and mImageValue on the main thread, and to perform
+ * mImageValue and mImageTracker on the main thread, and to perform
  * any necessary unlocking and untracking of the image.
  */
 class StyleImageRequestCleanupTask : public mozilla::Runnable
 {
 public:
   typedef nsStyleImageRequest::Mode Mode;
 
   StyleImageRequestCleanupTask(Mode aModeFlags,
@@ -1906,17 +1907,18 @@ public:
     , mRequestProxy(aRequestProxy)
     , mImageValue(aImageValue)
     , mImageTracker(aImageTracker)
   {
   }
 
   NS_IMETHOD Run() final
   {
-    MOZ_ASSERT(NS_IsMainThread());
+    MOZ_ASSERT(!mRequestProxy || NS_IsMainThread(),
+               "If mRequestProxy is non-null, we need to run on main thread!");
 
     if (!mRequestProxy) {
       return NS_OK;
     }
 
     if (mModeFlags & Mode::Track) {
       MOZ_ASSERT(mImageTracker);
       mImageTracker->Remove(mRequestProxy);
@@ -1927,17 +1929,25 @@ public:
     if (mModeFlags & Mode::Discard) {
       mRequestProxy->RequestDiscard();
     }
 
     return NS_OK;
   }
 
 protected:
-  virtual ~StyleImageRequestCleanupTask() { MOZ_ASSERT(NS_IsMainThread()); }
+  virtual ~StyleImageRequestCleanupTask()
+  {
+    MOZ_ASSERT(mImageValue->mRequests.Count() == 0 || NS_IsMainThread(),
+               "If mImageValue has any mRequests, we need to run on main "
+               "thread to release ImageValues!");
+    MOZ_ASSERT((!mRequestProxy && !mImageTracker) || NS_IsMainThread(),
+               "mRequestProxy and mImageTracker's destructor need to run "
+               "on the main thread!");
+  }
 
 private:
   Mode mModeFlags;
   // Since we always dispatch this runnable to the main thread, these will be
   // released on the main thread when the runnable itself is released.
   RefPtr<imgRequestProxy> mRequestProxy;
   RefPtr<css::ImageValue> mImageValue;
   RefPtr<ImageTracker> mImageTracker;
@@ -1981,35 +1991,40 @@ nsStyleImageRequest::~nsStyleImageReques
   // up, we must untrack and unlock the image (depending on mModeFlags),
   // and release mRequestProxy and mImageValue, all on the main thread.
   {
     RefPtr<StyleImageRequestCleanupTask> task =
         new StyleImageRequestCleanupTask(mModeFlags,
                                          mRequestProxy.forget(),
                                          mImageValue.forget(),
                                          mImageTracker.forget());
-    if (NS_IsMainThread()) {
+    if (NS_IsMainThread() || !IsResolved()) {
       task->Run();
     } else {
-      NS_DispatchToMainThread(task.forget());
+      MOZ_ASSERT(IsResolved() == bool(mDocGroup),
+                 "We forgot to cache mDocGroup in Resolve()?");
+      mDocGroup->Dispatch("StyleImageRequestCleanupTask",
+                          TaskCategory::Other, task.forget());
     }
   }
 
   MOZ_ASSERT(!mRequestProxy);
   MOZ_ASSERT(!mImageValue);
   MOZ_ASSERT(!mImageTracker);
 }
 
 bool
 nsStyleImageRequest::Resolve(nsPresContext* aPresContext)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!IsResolved(), "already resolved");
+  MOZ_ASSERT(aPresContext);
 
   mResolved = true;
+  mDocGroup = aPresContext->Document()->GetDocGroup();
 
   // For now, just have unique nsCSSValue/ImageValue objects.  We should
   // really store the ImageValue on the Servo specified value, so that we can
   // share imgRequestProxys that come from the same rule in the same
   // document.
   mImageValue->Initialize(aPresContext->Document());
 
   nsCSSValue value;
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -380,16 +380,19 @@ private:
   nsStyleImageRequest& operator=(const nsStyleImageRequest& aOther) = delete;
 
   void MaybeTrackAndLock();
 
   RefPtr<imgRequestProxy> mRequestProxy;
   RefPtr<mozilla::css::ImageValue> mImageValue;
   RefPtr<mozilla::dom::ImageTracker> mImageTracker;
 
+  // Cache DocGroup for dispatching events in the destructor.
+  RefPtr<mozilla::dom::DocGroup> mDocGroup;
+
   Mode mModeFlags;
   bool mResolved;
 };
 
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(nsStyleImageRequest::Mode)
 
 enum nsStyleImageType {
   eStyleImageType_Null,