Bug 1196476 - Replace ProgressTracker::FirstObserverIs() with a simpler mechanism on imgRequest. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 24 Aug 2015 19:49:33 -0700
changeset 259164 2e24f461affef38c5060648fe237a89ac080232f
parent 259163 ce19ea0695c2f5ea38fb3ecd7f2701bcaa993172
child 259165 8d286ac48d5daf4d0128db86e8a49afde9d07b8e
push id64149
push usermfowler@mozilla.com
push dateTue, 25 Aug 2015 02:50:01 +0000
treeherdermozilla-inbound@8d286ac48d5d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1196476
milestone43.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 1196476 - Replace ProgressTracker::FirstObserverIs() with a simpler mechanism on imgRequest. r=tn
image/ProgressTracker.cpp
image/ProgressTracker.h
image/imgRequest.cpp
image/imgRequest.h
--- a/image/ProgressTracker.cpp
+++ b/image/ProgressTracker.cpp
@@ -420,30 +420,16 @@ ProgressTracker::RemoveObserver(IProgres
   if (aObserver->NotificationsDeferred() && runnable) {
     runnable->RemoveObserver(aObserver);
     aObserver->SetNotificationsDeferred(false);
   }
 
   return removed;
 }
 
-bool
-ProgressTracker::FirstObserverIs(IProgressObserver* aObserver)
-{
-  MOZ_ASSERT(NS_IsMainThread(), "Use mObservers on main thread only");
-  ObserverArray::ForwardIterator iter(mObservers);
-  while (iter.HasMore()) {
-    nsRefPtr<IProgressObserver> observer = iter.GetNext().get();
-    if (observer) {
-      return observer.get() == aObserver;
-    }
-  }
-  return false;
-}
-
 void
 ProgressTracker::OnUnlockedDraw()
 {
   MOZ_ASSERT(NS_IsMainThread());
   NOTIFY_IMAGE_OBSERVERS(mObservers,
                          Notify(imgINotificationObserver::UNLOCKED_DRAW));
 }
 
--- a/image/ProgressTracker.h
+++ b/image/ProgressTracker.h
@@ -150,21 +150,16 @@ public:
   // with its loading progress. Weak pointers.
   void AddObserver(IProgressObserver* aObserver);
   bool RemoveObserver(IProgressObserver* aObserver);
   size_t ObserverCount() const {
     MOZ_ASSERT(NS_IsMainThread(), "Use mObservers on main thread only");
     return mObservers.Length();
   }
 
-  // This is intentionally non-general because its sole purpose is to support
-  // some obscure network priority logic in imgRequest. That stuff could
-  // probably be improved, but it's too scary to mess with at the moment.
-  bool FirstObserverIs(IProgressObserver* aObserver);
-
   // Resets our weak reference to our image. Image subclasses should call this
   // in their destructor.
   void ResetImage();
 
 private:
   typedef nsTObserverArray<mozilla::WeakPtr<IProgressObserver>> ObserverArray;
   friend class AsyncNotifyRunnable;
   friend class AsyncNotifyCurrentStateRunnable;
--- a/image/imgRequest.cpp
+++ b/image/imgRequest.cpp
@@ -59,16 +59,17 @@ NS_IMPL_ISUPPORTS(imgRequest,
                   nsIChannelEventSink,
                   nsIInterfaceRequestor,
                   nsIAsyncVerifyRedirectCallback)
 
 imgRequest::imgRequest(imgLoader* aLoader, const ImageCacheKey& aCacheKey)
  : mLoader(aLoader)
  , mCacheKey(aCacheKey)
  , mLoadId(nullptr)
+ , mFirstProxy(nullptr)
  , mValidator(nullptr)
  , mInnerWindowId(0)
  , mCORSMode(imgIRequest::CORS_NONE)
  , mReferrerPolicy(mozilla::net::RP_Default)
  , mImageErrorCode(NS_OK)
  , mMutex("imgRequest")
  , mProgressTracker(new ProgressTracker())
  , mIsMultiPartChannel(false)
@@ -213,16 +214,22 @@ imgRequest::ResetCacheEntry()
 }
 
 void
 imgRequest::AddProxy(imgRequestProxy* proxy)
 {
   NS_PRECONDITION(proxy, "null imgRequestProxy passed in");
   LOG_SCOPE_WITH_PARAM(GetImgLog(), "imgRequest::AddProxy", "proxy", proxy);
 
+  if (!mFirstProxy) {
+    // Save a raw pointer to the first proxy we see, for use in the network
+    // priority logic.
+    mFirstProxy = proxy;
+  }
+
   // If we're empty before adding, we have to tell the loader we now have
   // proxies.
   nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
   if (progressTracker->ObserverCount() == 0) {
     MOZ_ASSERT(mURI, "Trying to SetHasProxies without key uri.");
     if (mLoader) {
       mLoader->SetHasProxies(this);
     }
@@ -530,18 +537,17 @@ imgRequest::AdjustPriority(imgRequestPro
 {
   // only the first proxy is allowed to modify the priority of this image load.
   //
   // XXX(darin): this is probably not the most optimal algorithm as we may want
   // to increase the priority of requests that have a lot of proxies.  the key
   // concern though is that image loads remain lower priority than other pieces
   // of content such as link clicks, CSS, and JS.
   //
-  nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
-  if (!progressTracker->FirstObserverIs(proxy)) {
+  if (!mFirstProxy || proxy != mFirstProxy) {
     return;
   }
 
   nsCOMPtr<nsISupportsPriority> p = do_QueryInterface(mChannel);
   if (p) {
     p->AdjustPriority(delta);
   }
 }
--- a/image/imgRequest.h
+++ b/image/imgRequest.h
@@ -251,16 +251,20 @@ private:
   /* we hold on to this to this so long as we have observers */
   nsRefPtr<imgCacheEntry> mCacheEntry;
 
   /// The key under which this imgRequest is stored in the image cache.
   ImageCacheKey mCacheKey;
 
   void* mLoadId;
 
+  /// Raw pointer to the first proxy that was added to this imgRequest. Use only
+  /// pointer comparisons; there's no guarantee this will remain valid.
+  void* mFirstProxy;
+
   imgCacheValidator* mValidator;
   nsCOMPtr<nsIAsyncVerifyRedirectCallback> mRedirectCallback;
   nsCOMPtr<nsIChannel> mNewRedirectChannel;
 
   // The ID of the inner window origin, used for error reporting.
   uint64_t mInnerWindowId;
 
   // The CORS mode (defined in imgIRequest) this image was loaded with. By