Bug 1338446 Part 4 - Label StyleImageRequestCleanupTask. r=heycam
authorTing-Yu Lin <tlin@mozilla.com>
Tue, 14 Mar 2017 21:29:55 +0800
changeset 348429 27748ccce77e799a07f017e6ae1d4b1982ad5c28
parent 348428 c86e4966a1224fd6a66e23f686fcc0abd2ce8397
child 348430 21f9aecdc5d703c911fb93dfaa3b9fd579301a41
push id39160
push usertlin@mozilla.com
push dateMon, 20 Mar 2017 15:27:11 +0000
treeherderautoland@f018d6eefc41 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1338446
milestone55.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 1338446 Part 4 - Label StyleImageRequestCleanupTask. r=heycam 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,