Bug 1139804 (Part 4) - Stop accessing private imgRequest members in imgLoader. r=baku
authorSeth Fowler <seth@mozilla.com>
Mon, 23 Mar 2015 19:37:45 -0700
changeset 265539 06eca034b0850718b9cdf562df452bac08d29029
parent 265538 61c1f8d6b44a88781f49075c1d57e8ae1ea6ffbd
child 265540 e488bdbe1bc8496358d83afdcae14942e0e29ea6
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1139804
milestone39.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 1139804 (Part 4) - Stop accessing private imgRequest members in imgLoader. r=baku
image/src/imgLoader.cpp
image/src/imgRequest.h
--- a/image/src/imgLoader.cpp
+++ b/image/src/imgLoader.cpp
@@ -1441,34 +1441,34 @@ bool imgLoader::ValidateRequestWithNewCh
   // now we need to insert a new channel request object inbetween the real
   // request and the proxy that basically delays loading the image until it
   // gets a 304 or figures out that this needs to be a new request
 
   nsresult rv;
 
   // If we're currently in the middle of validating this request, just hand
   // back a proxy to it; the required work will be done for us.
-  if (request->mValidator) {
+  if (request->GetValidator()) {
     rv = CreateNewProxyForRequest(request, aLoadGroup, aObserver,
                                   aLoadFlags, aProxyRequest);
     if (NS_FAILED(rv)) {
       return false;
     }
 
     if (*aProxyRequest) {
       imgRequestProxy* proxy = static_cast<imgRequestProxy*>(*aProxyRequest);
 
       // We will send notifications from imgCacheValidator::OnStartRequest().
       // In the mean time, we must defer notifications because we are added to
       // the imgRequest's proxy list, and we can get extra notifications
       // resulting from methods such as RequestDecode(). See bug 579122.
       proxy->SetNotificationsDeferred(true);
 
       // Attach the proxy without notifying
-      request->mValidator->AddProxy(proxy);
+      request->GetValidator()->AddProxy(proxy);
     }
 
     return NS_SUCCEEDED(rv);
 
   } else {
     // We will rely on Necko to cache this request when it's possible, and to
     // tell imgCacheValidator::OnStartRequest whether the request came from its
     // cache.
@@ -1523,17 +1523,17 @@ bool imgLoader::ValidateRequestWithNewCh
       rv = corsproxy->Init(newChannel);
       if (NS_FAILED(rv)) {
         return false;
       }
 
       listener = corsproxy;
     }
 
-    request->mValidator = hvc;
+    request->SetValidator(hvc);
 
     // We will send notifications from imgCacheValidator::OnStartRequest().
     // In the mean time, we must defer notifications because we are added to
     // the imgRequest's proxy list, and we can get extra notifications
     // resulting from methods such as RequestDecode(). See bug 579122.
     req->SetNotificationsDeferred(true);
 
     // Add the proxy without notifying
@@ -1622,17 +1622,17 @@ bool imgLoader::ValidateEntry(imgCacheEn
 
   // If the request's loadId is the same as the aCX, then it is ok to use
   // this one because it has already been validated for this context.
   //
   // XXX: nullptr seems to be a 'special' key value that indicates that NO
   //      validation is required.
   //
   void *key = (void *)aCX;
-  if (request->mLoadId != key) {
+  if (request->LoadId() != key) {
     // If we would need to revalidate this entry, but we're being told to
     // bypass the cache, we don't allow this entry to be used.
     if (aLoadFlags & nsIRequest::LOAD_BYPASS_CACHE)
       return false;
 
     // Determine whether the cache aEntry must be revalidated...
     validateRequest = ShouldRevalidateEntry(aEntry, aLoadFlags, hasExpired);
 
@@ -1651,20 +1651,22 @@ bool imgLoader::ValidateEntry(imgCacheEn
   }
 #endif
 
   // We can't use a cached request if it comes from a different
   // application cache than this load is expecting.
   nsCOMPtr<nsIApplicationCacheContainer> appCacheContainer;
   nsCOMPtr<nsIApplicationCache> requestAppCache;
   nsCOMPtr<nsIApplicationCache> groupAppCache;
-  if ((appCacheContainer = do_GetInterface(request->mRequest)))
+  if ((appCacheContainer = do_GetInterface(request->GetRequest()))) {
     appCacheContainer->GetApplicationCache(getter_AddRefs(requestAppCache));
-  if ((appCacheContainer = do_QueryInterface(aLoadGroup)))
+  }
+  if ((appCacheContainer = do_QueryInterface(aLoadGroup))) {
     appCacheContainer->GetApplicationCache(getter_AddRefs(groupAppCache));
+  }
 
   if (requestAppCache != groupAppCache) {
     PR_LOG(GetImgLog(), PR_LOG_DEBUG,
            ("imgLoader::ValidateEntry - Unable to use cached imgRequest "
             "[request=%p] because of mismatched application caches\n",
             address_of(request)));
     return false;
   }
@@ -2517,17 +2519,17 @@ imgCacheValidator::imgCacheValidator(nsP
 {
   NewRequestAndEntry(forcePrincipalCheckForCacheEntry, loader, getter_AddRefs(mNewRequest),
                      getter_AddRefs(mNewEntry));
 }
 
 imgCacheValidator::~imgCacheValidator()
 {
   if (mRequest) {
-    mRequest->ClearValidator();
+    mRequest->SetValidator(nullptr);
   }
 }
 
 void imgCacheValidator::AddProxy(imgRequestProxy *aProxy)
 {
   // aProxy needs to be in the loadgroup since we're validating from
   // the network.
   aProxy->AddToLoadGroup();
@@ -2576,17 +2578,17 @@ NS_IMETHODIMP imgCacheValidator::OnStart
         // asynchronously-called function.
         proxy->SyncNotifyListener();
       }
 
       // We don't need to load this any more.
       aRequest->Cancel(NS_BINDING_ABORTED);
 
       mRequest->SetLoadId(mContext);
-      mRequest->ClearValidator();
+      mRequest->SetValidator(nullptr);
 
       mRequest = nullptr;
 
       mNewRequest = nullptr;
       mNewEntry = nullptr;
 
       return NS_OK;
     }
@@ -2609,17 +2611,17 @@ NS_IMETHODIMP imgCacheValidator::OnStart
 
   int32_t corsmode = mRequest->GetCORSMode();
   ReferrerPolicy refpol = mRequest->GetReferrerPolicy();
   nsCOMPtr<nsIPrincipal> loadingPrincipal = mRequest->GetLoadingPrincipal();
 
   // Doom the old request's cache entry
   mRequest->RemoveFromCache();
 
-  mRequest->ClearValidator();
+  mRequest->SetValidator(nullptr);
   mRequest = nullptr;
 
   // We use originalURI here to fulfil the imgIRequest contract on GetURI.
   nsCOMPtr<nsIURI> originalURI;
   channel->GetOriginalURI(getter_AddRefs(originalURI));
   mNewRequest->Init(originalURI, uri, aRequest, channel, mNewEntry,
                     mContext, loadingPrincipal,
                     corsmode, refpol);
--- a/image/src/imgRequest.h
+++ b/image/src/imgRequest.h
@@ -153,53 +153,55 @@ public:
   /// @return the priority of the underlying network request, or
   /// PRIORITY_NORMAL if it doesn't support nsISupportsPriority.
   int32_t Priority() const;
 
   /// Adjust the priority of the underlying network request by @aDelta on behalf
   /// of @aProxy.
   void AdjustPriority(imgRequestProxy* aProxy, int32_t aDelta);
 
+  /// Returns a weak pointer to the underlying request.
+  nsIRequest* GetRequest() const { return mRequest; }
+
   nsITimedChannel* GetTimedChannel() const { return mTimedChannel; }
 
   nsresult GetSecurityInfo(nsISupports** aSecurityInfoOut);
 
-  void ClearValidator() { mValidator = nullptr; }
+  imgCacheValidator* GetValidator() const { return mValidator; }
+  void SetValidator(imgCacheValidator* aValidator) { mValidator = aValidator; }
+
+  void* LoadId() const { return mLoadId; }
+  void SetLoadId(void* aLoadId) { mLoadId = aLoadId; }
 
-  void SetLoadId(void* aLoadId) { mLoadId = aLoadId; }
+  /// Reset the cache entry after we've dropped our reference to it. Used by
+  /// imgLoader when our cache entry is re-requested after we've dropped our
+  /// reference to it.
+  void SetCacheEntry(imgCacheEntry* aEntry);
+
+  /// Returns whether we've got a reference to the cache entry.
+  bool HasCacheEntry() const;
+
+  /// Set whether this request is stored in the cache. If it isn't, regardless
+  /// of whether this request has a non-null mCacheEntry, this imgRequest won't
+  /// try to update or modify the image cache.
+  void SetIsInCache(bool aCacheable);
 
   void EvictFromCache();
   void RemoveFromCache();
 
+  nsIProperties* Properties() const { return mProperties; }
+
 private:
-  friend class imgLoader;
   friend class mozilla::image::ProgressTracker;
 
   void Cancel(nsresult aStatus);
 
-  inline nsIProperties *Properties() {
-    return mProperties;
-  }
-
-  // Reset the cache entry after we've dropped our reference to it. Used by the
-  // imgLoader when our cache entry is re-requested after we've dropped our
-  // reference to it.
-  void SetCacheEntry(imgCacheEntry *entry);
-
-  // Returns whether we've got a reference to the cache entry.
-  bool HasCacheEntry() const;
-
   // Update the cache entry size based on the image container.
   void UpdateCacheEntrySize();
 
-  // Set whether this request is stored in the cache. If it isn't, regardless
-  // of whether this request has a non-null mCacheEntry, this imgRequest won't
-  // try to update or modify the image cache.
-  void SetIsInCache(bool cacheable);
-
   bool HasConsumers();
 
   /// Returns true if RequestDecode() was called.
   bool IsDecodeRequested() const;
 
 public:
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER